Skip to content

IA2: Do not treat huge base64 data as NVDA might freeze in Google Chrome (#10227) - Py2 backport to beta#10240

Merged
michaelDCurran merged 2 commits into
nvaccess:betafrom
accessolutions:i10227-ia2Base64-py2
Sep 24, 2019
Merged

IA2: Do not treat huge base64 data as NVDA might freeze in Google Chrome (#10227) - Py2 backport to beta#10240
michaelDCurran merged 2 commits into
nvaccess:betafrom
accessolutions:i10227-ia2Base64-py2

Conversation

@JulienCochuyt

Copy link
Copy Markdown
Contributor

Link to issue number:

Issue #10227
Backport of #10231 into beta

Summary of the issue:

Huge base64 embedded in the src attribute of a focusable img element can freeze Google Chrome.

Description of how this pull request fixes the issue:

When the IA2 attribute string exceeds a threshold of 4096 characters, truncate the base64 from the text passed to IAccessibleHandler.splitIA2Attribs before treating it.

Testing performed:

Known issues with pull request:

The huge base64 is still uselessly present in the Virtual Buffer data.
The NVDAHelper thus would probably benefit from an impact on that regard.

Change log entry:

Section: Bug fixes

NVDA no longer crashes in Google Chrome when an image contains huge base64 data.

@JulienCochuyt JulienCochuyt changed the title IA2: Do not treat huge base64 data as NVDA might freeze in Google Chrome (#10227) IA2: Do not treat huge base64 data as NVDA might freeze in Google Chrome (#10227) - Py2 backport to beta Sep 18, 2019
@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

cc @michaelDCurran

@michaelDCurran

Copy link
Copy Markdown
Member

As I feel the other pr is not yet complete, and there is no public real-life testcase to point to, and we really want to get 2019.2.1 out soon, I think I need to refuse this pr sorry.

@JulienCochuyt

JulienCochuyt commented Sep 24, 2019

Copy link
Copy Markdown
Contributor Author

@michaelDCurran I understand the schedule for 2019.2.1 is a matter of importance.
Please excuse my insistance: This release is expected to live quite long.
A user facing this freeze and holding on threshold would need to downgrade as far as 2017.4.
I have just attached an HTML example of the issue in #10249
Here it is for convenience: i10227.html.txt
(Please remove the .txt extension to open this .html file.)

I agree the proposed fix is not fully satisfactory as it misses the Gecko vbufBackend impact you mentioned in #10231 (comment)
Nevertheless, it still at least allows a regular end user not to get frozen.
Please reconsider your decision.

I'll try to work asap on the Gecko vbufBackend side, but I have other obligations towards my customers. Thus, this complementary part of the fix won't for sure be ready for 2019.2.1

@michaelDCurran

Copy link
Copy Markdown
Member

thank you for the testcase, that helps a great deal. I will take a look at it tomorrow, then I can better feel the actual impact.

@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

Thank you for your time and consideration.

@michaelDCurran michaelDCurran merged commit 5f79898 into nvaccess:beta Sep 24, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Sep 24, 2019
@michaelDCurran michaelDCurran modified the milestones: 2019.3, 2019.2.1 Sep 24, 2019
michaelDCurran added a commit that referenced this pull request Sep 24, 2019
@JulienCochuyt JulienCochuyt deleted the i10227-ia2Base64-py2 branch September 25, 2019 07:00
JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request Sep 26, 2019
… Google Chrome (PR nvaccess#10240)

Ensure to truncate base64 data only in the `src` attribute, and not in an eventual
text content (eg. in and inline editor).

Re: nvaccess#10231 (comment)
michaelDCurran pushed a commit that referenced this pull request Sep 30, 2019
… Google Chrome (PR #10240) (#10282)

* Fix up of: IA2: Do not treat huge base64 data as NVDA might freeze in Google Chrome (PR #10240)

Ensure to truncate base64 data only in the `src` attribute, and not in an eventual
text content (eg. in and inline editor).

Re: #10231 (comment)

* Fix up of bd35387: Include the comma after "base64" in the capturing group
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants