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

Add encode for asterisk in form fields. #2655

Closed
wants to merge 1 commit into
from

Conversation

2 participants
@aballano

According to RFC3986 point 2.2 and RFC1738 point 2.2

URIs include components and subcomponents that are delimited by
characters in the "reserved" set. These characters are called
"reserved" because they may (or may not) be defined as delimiters by
the generic syntax, by each scheme-specific syntax, or by the
implementation-specific syntax of a URI's dereferencing algorithm.

Although:

[...] only alphanumerics, the special characters "$-_.+!*'(),", and
reserved characters used for their reserved purposes may be used
unencoded within a URL.

Therefore as far as I understood is optional to encode some characters ("$-_.+!*'(),"), therefore following the logic that HttpUrl.FORM_ENCODE_SET already includes almost all of this characters, I think it would make sense to be consistent within this convention and thus include the asterisk symbol as well.

Please let me know if you agree with this and if the changes made are fine.
Thank you in advance.

@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Jun 24, 2016

Member

What do Chrome and Firefox do?

Member

swankjesse commented Jun 24, 2016

What do Chrome and Firefox do?

@aballano

This comment has been minimized.

Show comment
Hide comment
@aballano

aballano Jun 24, 2016

AFAIK Chrome is sending unencoded *, but the point is that this is merely a matter of consistency since encoding is optional for certain characters, so if you're already encoding some of them, why not all?

AFAIK Chrome is sending unencoded *, but the point is that this is merely a matter of consistency since encoding is optional for certain characters, so if you're already encoding some of them, why not all?

@swankjesse

This comment has been minimized.

Show comment
Hide comment
@swankjesse

swankjesse Jun 24, 2016

Member

Our goal is consistency with Chrome and Firefox. Encoding characters for the heck of it is not prudent.

Member

swankjesse commented Jun 24, 2016

Our goal is consistency with Chrome and Firefox. Encoding characters for the heck of it is not prudent.

@swankjesse swankjesse closed this Jun 25, 2016

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