Skip to content

Uri encode / decode #1733

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

Merged
merged 93 commits into from
Apr 17, 2021
Merged

Uri encode / decode #1733

merged 93 commits into from
Apr 17, 2021

Conversation

MaxGraey
Copy link
Member

@MaxGraey MaxGraey commented Mar 16, 2021

Implement following global functions:

  • decodeURI

  • encodeURI

  • decodeURIComponent

  • encodeURIComponent

  • I've read the contributing guidelines

@MaxGraey MaxGraey changed the title Uri encodes Uri encode / decode Mar 16, 2021
@dcodeIO
Copy link
Member

dcodeIO commented Apr 6, 2021

I find it hard to verify by hand that there isn't an OOB load or store somewhere. The test is using Rtrace, though, which indicates that there isn't, but I'm still a bit hesitant to blindly approve. I think we need a lot more comments here, for example

  • what the while/do combo does
  • why we need the i > org branch etc.
  • what are the tables
  • what's the meaning of a 0 or 1 in the tables
  • what's happening with surrogates
  • how does the size estimate work
  • why exactly an URIError is thrown

@ExE-Boss
Copy link

ExE-Boss commented Apr 6, 2021

@dcodeIO

  • why exactly an URIError is thrown

Because that’s what ECMA‑262 specifies:

Copy link
Member

@dcodeIO dcodeIO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on what we just talked about offline, I'd like to retract my objections and leave it up to you, @MaxGraey :)

@MaxGraey
Copy link
Member Author

MaxGraey commented Apr 14, 2021

Sure. I'll add more comments before merging

@MaxGraey MaxGraey requested a review from dcodeIO April 17, 2021 10:36
@MaxGraey MaxGraey merged commit 7d0690d into AssemblyScript:master Apr 17, 2021
@MaxGraey MaxGraey deleted the uri-encodes branch April 17, 2021 20:10
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.

3 participants