Skip to content

Conversation

@Nyoho
Copy link
Contributor

@Nyoho Nyoho commented Jan 7, 2015

Fix to use semicolons as separators for GET not for POST.

(cf. issue: Truncation of POST data on semicolon during param processing (unencoding?) #543)

A semicolon ';' should be used as a separator according to a W3.org recommendation
http://www.w3.org/TR/1999/REC-html401-19991224/appendix/notes.html#h-B.2.2

The commit 71c6911 was the one for only POST not for GET, but the test was written for GET, which is kind of a discrepancy.

Fix to use semicolons as separators for GET not for POST
A semicolon ';' should be used as a separator according to a W3.org recommendation
http://www.w3.org/TR/1999/REC-html401-19991224/appendix/notes.html#h-B.2.2

The following commit was for only POST not for GET, but the test is
written for GET, which is kind of a discrepancy.
Do not truncate POST data on `;`, closes rack#543
rack@71c6911
@tdtds
Copy link

tdtds commented Mar 23, 2015

👍

@spastorino
Copy link
Contributor

👍 /cc @raggi since you committed can you take a look at this one?

@raggi
Copy link
Member

raggi commented Apr 2, 2015

@spastorino all yours bro ❤️

spastorino added a commit that referenced this pull request Apr 6, 2015
@spastorino spastorino merged commit 6d978cb into rack:master Apr 6, 2015
eileencodes added a commit to eileencodes/rails that referenced this pull request Apr 6, 2015
Recently rack was changed to have a second argument on the `parse_query`
method (in rack/rack#781). Rails relies on this and it's `parse_query`
method was missing the second argument. I've bumped rack in the gemspec
until it can be released so that tests between Rails master and Rack
master pass.
eileencodes added a commit to eileencodes/rails that referenced this pull request Apr 6, 2015
Recently rack was changed to have a second argument on the `parse_query`
method (in rack/rack#781). Rails relies on this and it's `parse_query`
method was complaining aobut missing the second argument. I changed the
arguments to `*` so we don't have this issue in the future and I've bumped
rack in the gemspec until it can be released so that tests between Rails
master and Rack master pass.
@eileencodes
Copy link
Member

This caused a bug in Rails if running rails master with rack master because the method signature of parse_query didn't match.

Open PR for fix: rails/rails#19665

eileencodes added a commit to eileencodes/rails that referenced this pull request Apr 6, 2015
Recently rack was changed to have a second argument on the `parse_query`
method (in rack/rack#781). Rails relies on this and it's `parse_query`
method was complaining about missing the second argument. I changed the
arguments to `*` so we don't have this issue in the future.
@spastorino
Copy link
Contributor

👍

@spastorino
Copy link
Contributor

@eileencodes just in case did this d68b93a

@eileencodes
Copy link
Member

👍 thanks!

@Nyoho
Copy link
Contributor Author

Nyoho commented Apr 7, 2015

Thank you for merging this PR and fixing the bug d68b93a!

@dbussink
Copy link

Looks like this fix did not make it into 1.6.1? Should it also be fixed in the 1.6 stable branch?

@dbussink
Copy link

@spastorino Do you think this fix can also make it into the 1.6 branch? I checked but the new 1.6.2 doesn't seem to contain this fix.

@spastorino
Copy link
Contributor

/cc @tenderlove

@tenderlove
Copy link
Member

Yep, I'll backport this.

spastorino added a commit that referenced this pull request Jun 17, 2015
@dbussink
Copy link

@tenderlove Thanks!

@matthewd
Copy link
Contributor

Backporting this Did Not Go Well. Hopefully #899 will do better.

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.

8 participants