Skip to content

Commit

Permalink
Merge pull request #42598 from poorna2152/xml_attr_crash
Browse files Browse the repository at this point in the history 
Fix compiler crash on xml attribute access on non `xml:Element` values
  • Loading branch information
@MaryamZi
MaryamZi authored Aug 2, 2024
2 parents 7db2e07 + 1922024 commit 925535a
Show file tree
Hide file tree
Showing 6 changed files with 295 additions and 6 deletions.
3 changes: 2 additions & 1 deletion compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6380,7 +6380,7 @@ private void rewriteFieldBasedAccess(BLangFieldBasedAccess fieldAccessExpr) {
(BVarSymbol) fieldAccessExpr.symbol, false, fieldAccessExpr.isStoreOnCreation);
}
} else if (types.isLaxFieldAccessAllowed(refType)) {
if (!(varRefTypeTag == TypeTags.XML || varRefTypeTag == TypeTags.XML_ELEMENT)) {
if (!types.isAssignable(refType, symTable.xmlType)) {
if (varRefTypeTag == TypeTags.MAP &&
TypeTags.isXMLTypeTag(Types.getImpliedType(((BMapType) refType).constraint).tag)) {
result = rewriteExpr(rewriteLaxMapAccess(fieldAccessExpr));
Expand All @@ -6391,6 +6391,7 @@ private void rewriteFieldBasedAccess(BLangFieldBasedAccess fieldAccessExpr) {
fieldAccessExpr.expr = types.addConversionExprIfRequired(fieldAccessExpr.expr, symTable.jsonType);
targetVarRef = new BLangJSONAccessExpr(fieldAccessExpr.pos, fieldAccessExpr.expr, stringLit);
} else {
fieldAccessExpr.expr = types.addConversionExprIfRequired(fieldAccessExpr.expr, symTable.xmlType);
BLangInvocation xmlAccessInvocation = rewriteXMLAttributeOrElemNameAccess(fieldAccessExpr);
xmlAccessInvocation.setBType(fieldAccessExpr.getBType());
result = xmlAccessInvocation;
Expand Down
5 changes: 4 additions & 1 deletion ...na-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8666,6 +8666,9 @@ private BType getLaxFieldAccessType(BType exprType) {
return symTable.jsonType;
case TypeTags.XML:
case TypeTags.XML_ELEMENT:
case TypeTags.XML_COMMENT:
case TypeTags.XML_PI:
case TypeTags.XML_TEXT:
return symTable.stringType;
case TypeTags.MAP:
return ((BMapType) exprType).constraint;
Expand Down Expand Up @@ -8760,7 +8763,7 @@ private boolean accessCouldResultInError(BType bType) {
return false;
}

if (type.tag == TypeTags.XML) {
if (types.isAssignable(bType, symTable.xmlType)) {
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion ...allerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/Types.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ public BType checkType(Location pos,
public boolean isLaxFieldAccessAllowed(BType type) {
type = Types.getImpliedType(type);
Set<BType> visited = new HashSet<>();
return isLaxType(type, visited) == 1 || type.tag == TypeTags.XML || type.tag == TypeTags.XML_ELEMENT;
return isLaxType(type, visited) == 1 || isAssignable(type, symTable.xmlType);
}

// TODO : clean
Expand Down
76 changes: 73 additions & 3 deletions ...rina-unit-test/src/test/java/org/ballerinalang/test/types/xml/XMLAttributeAccessTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,69 @@ public void testGetAttrOfASequence() {
@Test
public void testXMLAttributeAccessNegative() {
CompileResult negative = BCompileUtil.compile("test-src/types/xml/xml-attribute-access-syntax-neg.bal");
Assert.assertEquals(negative.getErrorCount(), 2);
BAssertUtil.validateError(negative, 0, "invalid character ':' in field access expression", 7, 13);
BAssertUtil.validateError(negative, 1, "invalid character ':' in field access expression", 10, 13);
int index = 0;
BAssertUtil.validateError(negative, index++, "invalid character ':' in field access expression", 7, 13);
BAssertUtil.validateError(negative, index++, "invalid character ':' in field access expression", 10, 13);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml', found '(string|error)'", 17,
13);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml', found '(string|error)?'", 18,
13);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)'",
19, 16);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)?'",
20, 16);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml', found '(string|error)'", 23,
13);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml', found '(string|error)?'", 24,
13);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)'",
25, 16);
BAssertUtil.validateError(negative, index++,
"incompatible types: expected '(string|error)', found '(string|error)?'", 26, 22);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml?', found '(string|error)'", 29,
14);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml', found '(string|error)?'", 30,
13);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string?', found '(string|error)'",
31, 17);
BAssertUtil.validateError(negative, index++,
"incompatible types: expected '(string|error)', found '(string|error)?'", 32, 9);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml?', found '(string|error)'", 35,
14);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml', found '(string|error)?'", 36,
13);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)'",
37, 16);
BAssertUtil.validateError(negative, index++,
"incompatible types: expected '(string|error)', found '(string|error)?'", 38, 9);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml', found '(string|error)'", 41,
13);
BAssertUtil.validateError(negative, index++,
"invalid operation: type '(string|error)' does not support field access", 42, 13);
BAssertUtil.validateError(negative, index++,
"invalid operation: type '(string|error)' does not support optional field access", 43, 16);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)'",
49, 16);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string?', found '(string|error)?'",
50, 17);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)'",
53, 16);
BAssertUtil.validateError(negative, index++,
"incompatible types: expected '(string|error)', found '(string|error)?'", 54, 22);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)'",
57, 16);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string?', found '(string|error)?'",
58, 17);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string?', found '(string|error)'",
61, 17);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string?', found '(string|error)?'",
62, 17);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'string', found '(string|error)'",
65, 16);
BAssertUtil.validateError(negative, index++, "incompatible types: expected 'xml?', found '(string|error)?'", 66,
14);
Assert.assertEquals(negative.getErrorCount(), index);

}

@Test
Expand Down Expand Up @@ -114,6 +174,16 @@ public void testErrorsOnXMLIndexedOptionalAttributeAccess() {
BRunUtil.invoke(compileResult, "testErrorsOnXMLIndexedOptionalAttributeAccess");
}

@Test
public void testXmlAttributeAccessOnXmlUnionTypes() {
BRunUtil.invoke(compileResult, "testXmlAttributeAccessOnXmlUnionTypes");
}

@Test
public void testErrorsOnXmlAttributeAccessOnNonXmlElementValue() {
BRunUtil.invoke(compileResult, "testErrorsOnXmlAttributeAccessOnNonXmlElementValue");
}

@AfterClass
public void tearDown() {
compileResult = null;
Expand Down
55 changes: 55 additions & 0 deletions ...erina-unit-test/src/test/resources/test-src/types/xml/xml-attribute-access-syntax-neg.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,58 @@ function testXMLAttributeAccessWithNamespaceSyntaxNeg() {
var q = m.b:c;
}

type XC xml:Comment;

function testXmlAttributeAccessOnNonXmlElementValueNeg() {
xml:Comment x1 = xml `<!--This is a comment-->`;
xml _ = x1.attr;
xml _ = x1?.attr;
string _ = x1.attr;
string _ = x1?.attr;

xml:ProcessingInstruction x2 = xml `<?data?>`;
xml _ = x2.attr;
xml _ = x2?.attr;
string _ = x1.attr;
string|error r = x1?.attr;

xml:Text x3 = xml `This is an xml text`;
xml? _ = x3.attr;
xml _ = x3?.attr;
string? _ = x3.attr;
r = x3?.attr;

xml<xml:ProcessingInstruction|xml:Text> x4 = xml `<?pi?>text two<?data?>`;
xml? _ = x4.attr;
xml _ = x4?.attr;
string _ = x4.attr;
r = x4?.attr;

XC x5 = xml `<!--comment2-->`;
xml _ = x5.attr;
xml _ = x5.attr.attr2;
string _ = x5.attr?.attr3;
}


function testOptionalXmlAttributeAccessNeg() {
xml:Element x1 = xml `<x/>`;
string _ = x1.attr;
string? _ = x1?.attr;

XC x2 = xml `<!--comment-->`;
string _ = x2.attr;
string|error r = x1?.attr;

xml:ProcessingInstruction x3 = xml `<?data?>`;
string _ = x3.attr;
string? _ = x3?.attr;

xml:Text x4 = xml `xml text`;
string? _ = x4.attr;
string? _ = x4?.attr;

xml:Text|xml:Comment x5 = xml `<!--comment2-->`;
string _ = x5.attr;
xml? _ = x5?.attr;
}
160 changes: 160 additions & 0 deletions ...ballerina-unit-test/src/test/resources/test-src/types/xml/xml-attribute-access-syntax.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,166 @@ function testErrorsOnXMLIndexedOptionalAttributeAccess() {
assertError((x3/**/<f>/*)[0]?.id, errorMessage, expTextDetailMessage);
}

type XC xml:Comment;
type XPI xml:ProcessingInstruction;
type XT xml:Text;
type XE xml:Element;

function testXmlAttributeAccessOnXmlUnionTypes() {
xml<xml:Element|xml:Text> x1 = xml `<a attr="aa">a</a>`;
string|error result = x1.attr;
assertNonErrorValue(result, "aa");
string|error? resultOptional = x1?.attr;
assertNonErrorValue(resultOptional, "aa");

xml<xml:Element|xml:Comment> x2 = xml `<b attr="ba">b</b>`;
assertNonErrorValue(x2.attr, "ba");
assertNonErrorValue(x2?.attr, "ba");

xml<xml:Element|xml:ProcessingInstruction> x3 = xml `<c attr="ca">c</c>`;
assertNonErrorValue(x3.attr, "ca");
assertNonErrorValue(x3?.attr, "ca");

xml<xml:Element|xml:Text|xml:ProcessingInstruction> x4 = xml `<d attr="da">d</d>`;
assertNonErrorValue(x4.attr, "da");
assertNonErrorValue(x4?.attr, "da");

xml<xml:Element|xml:Comment|xml:ProcessingInstruction> x5 = xml `<e attr="ea">e</e>`;
result = x5.attr;
assertNonErrorValue(result, "ea");
resultOptional = x5?.attr;
assertNonErrorValue(resultOptional, "ea");

xml<xml:Element|xml:Comment|xml:Text> x6 = xml `<f attr="fa">f</f>`;
assertNonErrorValue(x6.attr, "fa");
assertNonErrorValue(x6?.attr, "fa");

xml<xml:Element|xml:ProcessingInstruction|xml:Comment|xml:Text> x7 = xml `<g attr="ga">g</g>`;
result = x7.attr;
assertNonErrorValue(result, "ga");
resultOptional = x7?.attr;
assertNonErrorValue(resultOptional, "ga");

xml:Element|xml:ProcessingInstruction x8 = xml `<h attr="ha">h</h>`;
assertNonErrorValue(x8.attr, "ha");
assertNonErrorValue(x8?.attr, "ha");

xml:Element|xml:Text x9 = xml `<j attr="ja">j</j>`;
result = x9.attr;
assertNonErrorValue(result, "ja");
resultOptional = x9?.attr;
assertNonErrorValue(resultOptional, "ja");

xml:Element|xml:Comment x10 = xml `<k attr="ka">k</k>`;
assertNonErrorValue(x10.attr, "ka");
assertNonErrorValue(x10?.attr, "ka");

XC|XE x11 = xml `<l attr="la">l</l>`;
result = x11.attr;
assertNonErrorValue(result, "la");
resultOptional = x11?.attr;
assertNonErrorValue(resultOptional, "la");
}

function testErrorsOnXmlAttributeAccessOnNonXmlElementValue() {
string errorMessage = "{ballerina/lang.xml}XMLOperationError";

xml:Comment x1 = xml `<!--comment-->`;
string|error result = x1.attr;
assertError(result, errorMessage, "invalid xml attribute access on xml comment");
string|error? resultOptional = x1?.attr;
assertError(resultOptional, errorMessage, "invalid xml attribute access on xml comment");

xml:Text x2 = xml `text`;
assertError(x2.attr, errorMessage, "invalid xml attribute access on xml text");
assertError(x2?.attr, errorMessage, "invalid xml attribute access on xml text");

xml:ProcessingInstruction x3 = xml `<?data?>`;
assertError(x3.attr, errorMessage, "invalid xml attribute access on xml pi");
assertError(x3?.attr, errorMessage, "invalid xml attribute access on xml pi");

xml<xml:Comment|xml:Text> x4 = xml `<!--comment-->text`;
result = x4.attr;
assertError(result, errorMessage, "invalid xml attribute access on xml sequence");
resultOptional = x4?.attr;
assertError(resultOptional, errorMessage, "invalid xml attribute access on xml sequence");

xml<xml:Comment|xml:ProcessingInstruction> x5 = xml `<?data?><!--comment-->`;
assertError(x5.attr, errorMessage, "invalid xml attribute access on xml sequence");
assertError(x5?.attr, errorMessage, "invalid xml attribute access on xml sequence");

xml<xml:Text|xml:ProcessingInstruction> x6 = xml `<?data?>text`;
assertError(x6.attr, errorMessage, "invalid xml attribute access on xml sequence");
assertError(x6?.attr, errorMessage, "invalid xml attribute access on xml sequence");

xml<xml:Text|xml:ProcessingInstruction|xml:Comment> x7 = xml `<!--comment--><?data?>text`;
result = x7.attr;
assertError(result, errorMessage, "invalid xml attribute access on xml sequence");
resultOptional = x7?.attr;
assertError(resultOptional, errorMessage, "invalid xml attribute access on xml sequence");

xml<xml:Element|xml:Text> x8 = xml `text`;
assertError(x8.attr, errorMessage, "invalid xml attribute access on xml text");
assertError(x8?.attr, errorMessage, "invalid xml attribute access on xml text");

xml<xml:Element|xml:Comment> x9 = xml `<!--comment-->`;
assertError(x9.attr, errorMessage, "invalid xml attribute access on xml comment");
assertError(x9?.attr, errorMessage, "invalid xml attribute access on xml comment");

xml<xml:Element|xml:ProcessingInstruction> x10 = xml `<?target?>`;
assertError(x10.attr, errorMessage, "invalid xml attribute access on xml pi");
assertError(x10?.attr, errorMessage, "invalid xml attribute access on xml pi");

xml<xml:Element|xml:Text> x11 = xml `<a attr="attr">a</a>text`;
assertError(x11.attr, errorMessage, "invalid xml attribute access on xml sequence");
assertError(x11?.attr, errorMessage, "invalid xml attribute access on xml sequence");

xml<xml:Element|xml:Comment> x12 = xml `<a attr="attr">a</a><!--comment-->`;
assertError(x12.attr, errorMessage, "invalid xml attribute access on xml sequence");
assertError(x12?.attr, errorMessage, "invalid xml attribute access on xml sequence");

xml<xml:Element|xml:ProcessingInstruction> x13 = xml `<a attr="attr">a</a><?target?>`;
assertError(x13.attr, errorMessage, "invalid xml attribute access on xml sequence");
assertError(x13?.attr, errorMessage, "invalid xml attribute access on xml sequence");

xml<xml:Element|xml:ProcessingInstruction|xml:Comment|xml:Text> x14 =
xml `<a attr="attr">a</a><?target?><!--comment-->text`;
assertError(x14.attr, errorMessage, "invalid xml attribute access on xml sequence");
assertError(x14?.attr, errorMessage, "invalid xml attribute access on xml sequence");

XC x15 = xml `<!--comment-->`;
assertError(x15.attr, errorMessage, "invalid xml attribute access on xml comment");
assertError(x15?.attr, errorMessage, "invalid xml attribute access on xml comment");

XPI x16 = xml `<?data?>`;
result = x16.attr;
assertError(result, errorMessage, "invalid xml attribute access on xml pi");
resultOptional = x16?.attr;
assertError(resultOptional, errorMessage, "invalid xml attribute access on xml pi");

XT x17 = xml `text`;
assertError(x17.attr, errorMessage, "invalid xml attribute access on xml text");
assertError(x17?.attr, errorMessage, "invalid xml attribute access on xml text");

xml:Comment|xml:Text x18 = xml `<!--comment-->`;
assertError(x18.attr, errorMessage, "invalid xml attribute access on xml comment");
assertError(x18?.attr, errorMessage, "invalid xml attribute access on xml comment");

xml:Text|xml:ProcessingInstruction x19 = xml `text`;
result = x19.attr;
assertError(result, errorMessage, "invalid xml attribute access on xml text");
resultOptional = x19.attr;
assertError(resultOptional, errorMessage, "invalid xml attribute access on xml text");

xml:ProcessingInstruction|xml:Comment x20 = xml `<?data?>`;
assertError(x20.attr, errorMessage, "invalid xml attribute access on xml pi");
assertError(x20?.attr, errorMessage, "invalid xml attribute access on xml pi");

xml:ProcessingInstruction|xml:Element x21 = xml `<?target?>`;
assertError(x21.attr, errorMessage, "invalid xml attribute access on xml pi");
assertError(x21?.attr, errorMessage, "invalid xml attribute access on xml pi");
}

function assertNonErrorValue(anydata|error actual, anydata expected) {
if actual is error {
panic error("expected [" + expected.toString() + "] " + ", but got error with message " + actual.message());
Expand Down

0 comments on commit 925535a

Please sign in to comment.