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

POST data included in signature when content type is "multipart/form-data" #141

Open
GoogleCodeExporter opened this issue Mar 1, 2016 · 4 comments

Comments

@GoogleCodeExporter
Copy link

What steps will reproduce the problem?
1. Create a post with the content type/encoding "multipart/form-data" with at 
least one posted variable
2. Echo the signatureBaseString in the OAuthRequest Library

What is the expected output?
You will see the variable included the signatureBaseString

What do you see instead?
Variables passed via "multipart/form-data", should not be included in the 
signatureBaseString.


Reference to the specification: Section 9.1.1. Normalize Request Parameters of 
OAuth 1.0a http://oauth.net/core/1.0a/#anchor13

Currently in the code there is a TODO marker for correct handling of the 
situation of "multipart/form-data", which does fetch the POST data.

Attached is a minor patch which no longer retrieves the POST data with a 
Comment so that it is clear that this behaviour is intentional.

Original issue reported on code.google.com by mat...@gmail.com on 25 Nov 2013 at 3:02

Attachments:

@GoogleCodeExporter
Copy link
Author

I see that that behavior was changed at some point, in the code there is a 
comment:

>  Get the body of a POST with multipart/form-data by Edison tsai on 16:52 
2010/09/16

The original code was using a separate signature for the request body, per 
discussion with Eran et al.
See also https://groups.google.com/forum/#!topic/oauth/rNA2R0ZJQCo
And http://www.marcworrell.com/article-3130-en.html

Maybe you can check the reasons behind Edison Tsai's commit?

Original comment by ma...@pobox.com on 25 Nov 2013 at 3:36

@GoogleCodeExporter
Copy link
Author

Thanks for finding the previous discussions: 
https://groups.google.com/forum/#!topic/oauth/rNA2R0ZJQCo
And http://www.marcworrell.com/article-3130-en.html

However they seem to be about allowing extended signing of body content above 
and beyond oauth standard signing. They were implemented in a separate class 
OAuthRequestVerifier.php in the verifyExtended method. These discussions were 
had in April 2008. The check-in from Edison Tsai seems to be unrelated to the 
extended verification.

The check-in looks to be a helpful change from what was there, which just throw 
an exception. There doesn't seem to be a lot of information about the Issue 81 
or commit 165. However it does not conform to the specs.

Would you be happy with some sort of flag that would make the behaviour act as 
it does currently and then apply the change?

Original comment by mat...@gmail.com on 25 Nov 2013 at 10:29

@GoogleCodeExporter
Copy link
Author

Sounds good to me. Maybe the best default behavior is to stick with the specs, 
and an option to switch to Tsai's additions.

Original comment by ma...@pobox.com on 26 Nov 2013 at 8:44

@GoogleCodeExporter
Copy link
Author

Finally found time to create the updated patch.

The flag by default is in-line with the spec.

The backwards compatibility maybe enabled by passing to the OAuthServer options 
parameter:
array (
'sign_body_of_multipart_request' => true
)

Looking at the code this was the cleanest way I could see that fits with the 
coding style/conventions.

Hope this is acceptable

Original comment by mat...@gmail.com on 3 Dec 2013 at 3:54

Attachments:

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

No branches or pull requests

1 participant