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 on XML parsing #764

Closed
bootstraponline opened this issue May 4, 2020 · 6 comments · Fixed by #771
Closed

Crash on XML parsing #764

bootstraponline opened this issue May 4, 2020 · 6 comments · Fixed by #771
Assignees
Labels
Milestone

Comments

@bootstraponline
Copy link
Contributor

Posted by Andrey Mischenko on Slack. Flank version unknown.

https://firebase-community.slack.com/archives/C1MTSQ5QT/p1588569925404500

We got invalid XML on one of our tests from test lab, it’s because Firebase converts some characters in failure message to html entities, which is invalid XML, which cause fails of our CI when try to get results
Invalid XML looks like:

<testcase name="test_name" classname="some.class.Name" time="23.194">
    <failure>java.net.ConnectException: Failed to connect to ... at &#8;&#8;&#8;(Coroutine boundary.&#8;(&#8;)

In original message we do not have html entities, just UTF character for Backspace https://unicode-table.com/en/0008/

I think it will be solved if Firebase will not encode html entities, or encode it using xml encoding using <!Entity> tag, or just replace & with & it will not be perfect, but better than broken xml parsing (edited)

@bootstraponline bootstraponline added this to the May 2020 milestone May 4, 2020
@gildor
Copy link
Contributor

gildor commented May 5, 2020

It's not a bug of Flank itself. Of course, Flank may fix it by sanitizing XML before attempt to parse it, but not sure that it worth it. But it's a very unfortunate bug of Firebase

@bootstraponline
Copy link
Contributor Author

I think we should preprocess the XML before parsing. Flank is use to working around weird FTL bugs. 😄

@gildor
Copy link
Contributor

gildor commented May 6, 2020

Flank version unknown

It's reproducible for me on flankVersion = "8.1.0", also it causes infinite hanging of test execution:

test_firebase_testlab - #459 Aborted after 3 days 20 hr 😬

But as it was said, it should be fixed already for snapshot versions of flank

@bootstraponline
Copy link
Contributor Author

I think the infinite hang is fixed but I'm not sure we fixed the XML parsing crash.

@gildor
Copy link
Contributor

gildor commented May 6, 2020

Yes, I meant that hang is fixed

@pawelpasterz
Copy link
Contributor

test_firebase_testlab - #459 Aborted after 3 days 20 hr 😬

Oh mann....

adamfilipow92 added a commit that referenced this issue May 7, 2020
Fix crash on parse some control chars
adamfilipow92 added a commit that referenced this issue May 7, 2020
detekt issues fix
adamfilipow92 added a commit that referenced this issue May 7, 2020
Fix crash on parse some control chars
adamfilipow92 added a commit that referenced this issue May 7, 2020
detekt issues fix
adamfilipow92 added a commit that referenced this issue May 8, 2020
Fix crash on parse some control chars
adamfilipow92 added a commit that referenced this issue May 8, 2020
detekt issues fix
adamfilipow92 added a commit that referenced this issue May 8, 2020
adamfilipow92 added a commit that referenced this issue May 8, 2020
* #764

Fix crash on parse some control chars

* #764 detekt issues fix

detekt issues fix

* Asserts in tests

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

* Add documentation of magicial numers

* set name of UtfControlChars enum to UtfControlCharRanges

* #764

Fix crash on parse some control chars

* #764 detekt issues fix

detekt issues fix

* Asserts in tests

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

* Add documentation of magicial numers

* set name of UtfControlChars enum to UtfControlCharRanges

* Detekt suggestions

* Create should_exists.txt

* #764

Fix crash on parse some control chars

* #764 detekt issues fix

detekt issues fix

* Asserts in tests

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

* Add documentation of magicial numers

* set name of UtfControlChars enum to UtfControlCharRanges

* Fail fast when results-dir is incorrect (#772)

* Fail fast when results-dir is incorrect

* #764 Change XmlPreprocessor more functional and

remove should_exists

* Add info about issue to release notes

Co-authored-by: Adam <[email protected]>
Co-authored-by: Jan Góral <[email protected]>
adamfilipow92 added a commit that referenced this issue May 13, 2020
* #764

Fix crash on parse some control chars

* #764 detekt issues fix

detekt issues fix

* Asserts in tests

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

* Add documentation of magicial numers

* set name of UtfControlChars enum to UtfControlCharRanges

* #764

Fix crash on parse some control chars

* #764 detekt issues fix

detekt issues fix

* Asserts in tests

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

* Add documentation of magicial numers

* set name of UtfControlChars enum to UtfControlCharRanges

* Detekt suggestions

* Create should_exists.txt

* #764

Fix crash on parse some control chars

* #764 detekt issues fix

detekt issues fix

* Asserts in tests

Add asserts in test and replace Files.readString to Files.readAllBytes for compability with java 1.8

* Add documentation of magicial numers

* set name of UtfControlChars enum to UtfControlCharRanges

* Fail fast when results-dir is incorrect (#772)

* Fail fast when results-dir is incorrect

* #764 Change XmlPreprocessor more functional and

remove should_exists

* Add info about issue to release notes

Co-authored-by: Adam <[email protected]>
Co-authored-by: Jan Góral <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants