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

Fix FormAuthenticator corrupting post parameters. #2320

Merged

Conversation

albfernandez
Copy link
Contributor

Fixes #2318

@albfernandez
Copy link
Contributor Author

I've provided a small test application #2318.

I would like to provide a JUnit test case but I'm having troubles setting up "environment" (I don't know how to setup Globals.setDefaultHabitat in a test envirnoment)

@Cousjava
Copy link
Contributor

jenkins test please

@arjantijms
Copy link
Contributor

arjantijms commented Jan 25, 2018

Hi, thanks for discovering and reporting this! Before we can accept the PR you must have signed the CLA, or did you already sign it?

As for the test; since one of the parameters will be corrupted, I guess it will show as corrupted just as well in a regular Servlet application, right?

The easiest way for us to test that it's broken in version X and really fixed in X+1 is by means of the Java EE 7 samples test. In this case, it looks like it's only an extra test method in the existing test here: https://github.com/javaee-samples/javaee7-samples/tree/master/servlet/security-form-based

Would be ace if you could do a PR at that repo too with the test demonstrating the issue. Thanks in advance!

@payara-ci
Copy link
Contributor

Quick build and test passed!

@Cousjava Cousjava added the PR: Awaiting CLA Contributor does not have a CLA or has submitted an unconfirmed CLA. label Jan 25, 2018
@albfernandez
Copy link
Contributor Author

CLA signed and submitted.

I'll make the pull request to javaee7-sameples with the test case.

@michaelranaldo michaelranaldo added PR: CLA CLA submitted on PR by the contributor and removed PR: Awaiting CLA Contributor does not have a CLA or has submitted an unconfirmed CLA. labels Jan 26, 2018
@albfernandez
Copy link
Contributor Author

albfernandez commented Jan 26, 2018

I've tried to reproduce the bug in javaee7-samples, but I've got a working test working fine: without the bug.

I've commited my work to my forked repo:

https://github.com/albfernandez/javaee7-samples/tree/formbased_authentication_corrupts_post_params_payara/servlet/security-form-based

I'll continue investigating next week.

@albfernandez
Copy link
Contributor Author

I've finally "fixed" the test and I've done the PR to javaee7-samples repository

javaee-samples/javaee7-samples#429

@arjantijms
Copy link
Contributor

@albfernandez thx a lot!

@arjantijms arjantijms added this to the Payara 4.181 milestone Jan 30, 2018
@arjantijms arjantijms merged commit 8b5156c into payara:master Jan 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: CLA CLA submitted on PR by the contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants