Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade com.ibm.icu:icu4j to latest (69.1) to avoid potential vulnerabilities it brings #3261

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

XenoAmess
Copy link
Contributor

Also change the template to suite this upgrade, as this upgrade makes the java class larger than jvm can accept.

@XenoAmess XenoAmess changed the title Upgrade com.ibm.icu:icu4j to latest (69.1) to avoid vulnerabilities it brings Upgrade com.ibm.icu:icu4j to latest (69.1) to avoid potential vulnerabilities it brings Aug 25, 2021
@XenoAmess XenoAmess force-pushed the upgrade_icu4j branch 5 times, most recently from c19532a to 10c0520 Compare August 25, 2021 16:20
@parrt parrt added the unicode label Oct 14, 2021
@parrt
Copy link
Member

parrt commented Oct 14, 2021

Looks like some errors in the Java tests...

@XenoAmess
Copy link
Contributor Author

XenoAmess commented Oct 15, 2021

Looks like some errors in the Java tests...

@parrt

I don't think the errors are from this pr

will rebase it on latest master and try again.


Oops seems I be wrong. I will find why.

…bilities it brings

Also change the template to suite this upgrade, as this upgrade makes the java class larger than jvm can accept.
@mmuth
Copy link

mmuth commented Oct 26, 2021

thanks for opening this PR @XenoAmess , we also wait for the merge :)

@parrt parrt merged commit 66eeafb into antlr:master Oct 26, 2021
@parrt
Copy link
Member

parrt commented Oct 26, 2021

merged manually

@parrt parrt added this to the 4.9.3 milestone Oct 26, 2021
@parrt
Copy link
Member

parrt commented Oct 26, 2021

I'm getting an error. Is this new @XenoAmess ?

java.lang.AssertionError: U+1F481 INFORMATION DESK PERSON is an emoji modifier base

	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.antlr.v4.test.tool.TestUnicodeData.testEnumeratedPropertyEquals(TestUnicodeData.java:143)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:258)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runners.Suite.runChild(Suite.java:128)
	at org.junit.runners.Suite.runChild(Suite.java:27)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)

@KvanTTT
Copy link
Member

KvanTTT commented Oct 27, 2021

Also, there are other weird errors after merging:

[ERROR] error(174): org\antlr\v4\test\runtime\java\api\perf\graphemes.g4:14:4: string literals and sets cannot be empty: []
[ERROR] C:\projects\antlr4\org\antlr\v4\test\runtime\java\api\perf\graphemes.g4 [14:4]: string literals and sets cannot be empty: []
[ERROR] error(174): org\antlr\v4\test\runtime\java\api\perf\graphemes.g4:14:77: string literals and sets cannot be empty: []
[ERROR] C:\projects\antlr4\org\antlr\v4\test\runtime\java\api\perf\graphemes.g4 [14:77]: string literals and sets cannot be empty: []

@KvanTTT
Copy link
Member

KvanTTT commented Oct 27, 2021

It seems like the following graphemes are not supported in the latest version of com.ibm.icu:icu4j:

A: [\p{Grapheme_Cluster_Break=E_Base}];

UnicodeData.getPropertyCodePoints just returns empty set for such a grapheme. That's why errors about empty string literals and sets are correct.

I suggest dropping these graphemes from test data if they became unsafe.

@parrt
Copy link
Member

parrt commented Oct 27, 2021

Agreed. it looks like I should simply drop the entire rule and then remove references to EmojiModifierSequence in the other rules like:

EmojiCoreSequence:
    EmojiModifierSequence
  | EmojiCombiningSequence
  | EmojiFlagSequence;

does that sound correct?

parrt added a commit to parrt/antlr4 that referenced this pull request Oct 27, 2021
parrt added a commit to parrt/antlr4 that referenced this pull request Oct 27, 2021
@parrt parrt mentioned this pull request Oct 27, 2021
@parrt
Copy link
Member

parrt commented Oct 27, 2021

Fixed with #3323

@KvanTTT
Copy link
Member

KvanTTT commented Oct 28, 2021

Maybe it makes sense to add some sort of error about unsupported graphemes. Because the message string literals and sets cannot be empty: [] is not clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants