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.

Sign up for GitHub

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

Jump to bottom

Refactor all "Magic Numbers" in the baseline to use ESAPI.properties configurations #403

Closed
xeno6696 opened this issue Jul 16, 2017 · 10 comments
Closed

Refactor all "Magic Numbers" in the baseline to use ESAPI.properties configurations #403

xeno6696 opened this issue Jul 16, 2017 · 10 comments

Comments

@xeno6696
Copy link
Collaborator

Also includes removing all deprecated security configuration calls.
@xeno6696 xeno6696 mentioned this issue Jul 16, 2017
Merged
xeno6696 added a commit that referenced this issue Jul 25, 2017
@xeno6696
Merge pull request #405 from xeno6696/develop
4f5daa9 
Multiple issues, #403, #400, #383, #405
xeno6696 added a commit that referenced this issue Jul 29, 2017
@xeno6696
Merge pull request #410 from xeno6696/develop
df944cf 
Issue #292 && Issue #403 -- Updated default regex size for jsessionid…
@xeno6696
Copy link
Collaborator Author

Using the regex \(.*[0-9]+.*\) to find the rest of these.

@kwwall
Copy link
Contributor

kwwall commented Jul 29, 2017 via email

@xeno6696
Copy link
Collaborator Author

hardcoded numbers in crypto? :-D

image

@kwwall
Copy link
Contributor

kwwall commented Jul 29, 2017 via email

@kwwall
Copy link
Contributor

kwwall commented Jul 29, 2017 via email

@xeno6696
Copy link
Collaborator Author

Jokes aside, just to be clear, I maybe misnamed the issue. I'm hunting for any numbers that ought to be configurable.. not necessarily wrapped into well-named constants. looks at the codecs yeah, I'm fine with char values in there...

@xeno6696
Copy link
Collaborator Author

I've run into a host of nasty issues with unit tests after refactoring the crypto classes. @kwwall , how much longer do you have on the crypto changes?

@kwwall
Copy link
Contributor

kwwall commented Jul 30, 2017 via email

@xeno6696
Copy link
Collaborator Author

Well, in regards to your email, I was scratching my head that the crypto unit tests were failing because they were grabbing Strings from the recommended SecurityConfiguration.getXProp() instead of from the deprecated methods. At any rate, duly rolled back. So at this point the only changes that will be left are the magic numbers. We can worry about getting rid of the deprecated methods later.

(FWIW: I had planned on delaying the merge until after you had pushed your crypto changes, but it sounds like the wiser path is just to have you handle the deprecation when you're ready. )

xeno6696 added a commit that referenced this issue Jul 30, 2017
@xeno6696
Merge pull request #411 from xeno6696/develop
d9cff08 
Merging commits related to #403.
@xeno6696
Copy link
Collaborator Author

Magic numbers are taken care of. Ditching the part of the ticket that tacked on removing deprecated methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kwwall @xeno6696