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

Crash in org.snakeyaml.engine.v2.api.LoadSettingsBuilder #3889

Closed
westnordost opened this issue Mar 19, 2022 · 10 comments · Fixed by #3914
Closed

Crash in org.snakeyaml.engine.v2.api.LoadSettingsBuilder #3889

westnordost opened this issue Mar 19, 2022 · 10 comments · Fixed by #3914
Assignees
Labels
blocked blocked by another issue bug

Comments

@westnordost
Copy link
Member

There is a reproducible crash for org.snakeyaml.engine.v2.api.LoadSettingsBuilder (dependency of kaml).

java.lang.ExceptionInInitializerError
at org.snakeyaml.engine.v2.api.LoadSettingsBuilder.<init>(LoadSettingsBuilder.java:66)
at org.snakeyaml.engine.v2.api.LoadSettings.builder(LoadSettings.java:79)
at com.charleskorn.kaml.YamlParser.<init>(YamlParser.kt:33)
at com.charleskorn.kaml.Yaml.decodeFromReader(Yaml.kt:51)
at com.charleskorn.kaml.Yaml.decodeFromStream(Yaml.kt:47)
at de.westnordost.streetcomplete.ktx.ResourcesKt.getYamlStringMap(Resources.kt:16)
at de.westnordost.streetcomplete.about.ChangelogFragmentKt$readChangelog$2.invokeSuspend(ChangelogFragment.kt:93)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
at kotlinx.coroutines.internal.LimitedDispatcher.run(LimitedDispatcher.kt:39)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:95)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
Caused by: java.util.regex.PatternSyntaxException: Syntax error in regexp pattern near index 13:
^\$\{\s*((?<name>\w+)((?<separator>:?(-|\?))(?<value>\w+)?)?)\s*\}$
^
at java.util.regex.Pattern.compileImpl(Native Method)
at java.util.regex.Pattern.compile(Pattern.java:411)
at java.util.regex.Pattern.<init>(Pattern.java:394)
at java.util.regex.Pattern.compile(Pattern.java:381)
at org.snakeyaml.engine.v2.resolver.JsonScalarResolver.<clinit>(JsonScalarResolver.java:41)
... 15 more

The crash reports I got so far were both from Android 5.1 devies and both (so far) from people who installed the app from F-Droid.

cc @FloEdelmann

@FloEdelmann
Copy link
Member

The problem seems to be the use of named capturing groups in the regex: https://stackoverflow.com/questions/54861220/regex-pattern-error-on-api-21android-5-and-below

I created an issue in snakeyaml: https://bitbucket.org/asomov/snakeyaml-engine/issues/34/patternsyntaxexception-due-to-named

@FloEdelmann FloEdelmann added the blocked blocked by another issue label Mar 19, 2022
@westnordost
Copy link
Member Author

Since this is caused by a dependency of a dependency, I expect this to take quite a while to fix - if the maintainer of snakeyaml actually recognizes it as an issue, that is.

As it is, StreetComplete is currently not working correctly for Android 5. Can you think of a workaround to not depend on snakeyaml? Maybe some older version of snakeyaml does not have this issue but still works fine with kaml and the older version of the dependency could be forced in our gradle? (I didn't find where actually the source code of snakeyaml is kept)

@FloEdelmann
Copy link
Member

FloEdelmann commented Mar 21, 2022

I didn't find where actually the source code of snakeyaml is kept

I was also confused by that, but the source code is in a different Bitbucket repository: https://bitbucket.org/snakeyaml/snakeyaml-engine/src/master/
And there is a GitHub mirror, too: https://github.com/snakeyaml/snakeyaml-engine

some older version of snakeyaml does not have this issue

This is the commit that introduced this particular regexp: 1ab4657 on Bitbucket / 1ab4657 on GitHub
According to the changelog, this was released in v2.1, so v2.0 should work fine (regarding this issue).

The latest kaml release that depends on that old version is v0.18.1 – we had to change a lot of stuff if we stuck to that version (if it would be possible at all), as we now use features last introduced in v0.42.0.

dependency could be forced in our gradle

I don't know if or how that would be possible in Gradle. But it seems it would not work anyway, since snakeyaml-engine v2.3 had a breaking API change that was accounted for in kaml v0.31.0.


So considering all of this, I only see three options:

  1. Wait (and hope) for the bug to be fixed, and disappointing Android 5 users
  2. Officially stop supporting Android 5
  3. Revert 9264654 and adf9a78 and Convert remaining Java files to Kotlin, switch to kaml for YAML parsing #3715 (at least temporarily)

@westnordost
Copy link
Member Author

The mantainer invited (us) to do a PR. Do you have time to do that? Otherwise I'd try because this is a kind of issue that we should try to fix before v42 moves out of beta.

@FloEdelmann
Copy link
Member

I don't have too much time right now, so please give it a go if you can :)

@westnordost westnordost self-assigned this Mar 22, 2022
@westnordost
Copy link
Member Author

I did the changes and posted a PR. The original issue seems to have vanished, https://bitbucket.org/asomov/snakeyaml-engine/issues is not accessible anymore and the PR vanished as well without me getting any notification about it.

But looking at https://bitbucket.org/snakeyaml/snakeyaml-engine/commits/ it looks my PR was accepted. BitBucket has a weird UX. I guess the maintainer removed the duplicate issue tracker behind the first link because here is another: https://bitbucket.org/snakeyaml/snakeyaml-engine/issues?status=new&status=open

@westnordost
Copy link
Member Author

Anyway, this is now blocked by waiting for a new release of snakeyaml. Comparing the releases on maven https://mvnrepository.com/artifact/org.snakeyaml/snakeyaml-engine with the recent commit history https://bitbucket.org/snakeyaml/snakeyaml-engine/commits/ looks like snakeyaml-engine is a project with an extremely slow release cycle. So I don't expect a new version soon.

@riQQ
Copy link
Collaborator

riQQ commented Mar 23, 2022

@FloEdelmann
Copy link
Member

FloEdelmann commented Mar 24, 2022

I guess the maintainer removed the duplicate issue tracker behind the first link because here is another

Confirmed by the library maintainer in https://bitbucket.org/snakeyaml/snakeyaml-engine/issues/34/support-https-githubcom-yaml-yaml-runtimes: "incorrectly resolved by PR which referenced issue 34 in an old repo"

So I don't expect a new version soon.

Something that might work instead:

  1. Add a dependency constraint, so that we can force a specific version of snakeyaml-engine, no matter what version kaml depends on: https://docs.gradle.org/current/userguide/dependency_constraints.html
  2. Download snakeyaml-engine from its git repository directly, so we can pin the version to the latest commit hash: https://stackoverflow.com/questions/18748436/is-it-possible-to-declare-git-repository-as-dependency-in-android-gradle

The commits since the last release look like they should be backwards-compatible, so kaml (expecting an older version) should probably continue to work fine. Only commit 08be4d2 could be a breaking change, but it could also just be an internal refactoring.

@westnordost
Copy link
Member Author

I was searching for that kind of option. I wasn't even able to create a jar with the checked out snakeyaml-engine repository (don't know how, I am only semi proficient with gradle, not maven). Maybe jitpack on the other hand can do that?

I also posted on their communication channel, who knows, maybe they are about to release a new version anyway or have a separate maven repository for the SNAPSHOT version: https://groups.google.com/g/snakeyaml-core/c/BbMICOmishM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked blocked by another issue bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants