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

Tomcat 8 MixedAuthenticator bug #192

Merged
merged 1 commit into from
Jan 22, 2015
Merged

Tomcat 8 MixedAuthenticator bug #192

merged 1 commit into from
Jan 22, 2015

Conversation

rripley
Copy link
Contributor

@rripley rripley commented Jan 21, 2015

For the Tomcat 8 MixedAuthenticator, a dummy LoginConfig object was being created and used instead of pulling LoginConfig out of the context.

being created and used instead of pulling LoginConfig out of the
context.
@hazendaz
Copy link
Member

Really good catch! Thank you for the contribution.

hazendaz added a commit that referenced this pull request Jan 22, 2015
@hazendaz hazendaz merged commit d24fa34 into Waffle:master Jan 22, 2015
@dblock
Copy link
Collaborator

dblock commented Jan 22, 2015

I encourage everyone to write tests for this kind of stuff!

@hazendaz
Copy link
Member

Sure. Makes perfect sense. In this case I did the original coding there. It was an api change in tomcat. I clearly dropped the original ball and appeared to have taken test code to create that object. I'll add some documentation for contributors if we don't have this as a requirement and I will not accept in future without test cases. Thanks for pointing this out.

--- Original Message ---

From: "Daniel Doubrovkine (dB.) @dblockdotorg" [email protected]
Sent: January 22, 2015 7:14 AM
To: "dblock/waffle" [email protected]
Cc: "Jeremy Landis" [email protected]
Subject: Re: [waffle] Tomcat 8 MixedAuthenticator bug (#192)

I encourage everyone to write tests for this kind of stuff!


Reply to this email directly or view it on GitHub:
#192 (comment)

@rripley rripley deleted the tomcat8_fix_LoginConfig branch January 22, 2015 17:35
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.

3 participants