support for variables in 'param-value' from properties file without depending on any 3rd party libs#106
support for variables in 'param-value' from properties file without depending on any 3rd party libs#106MadhbhavikaR wants to merge 9 commits into
Conversation
Enhancement - Properties file support for web.xml
|
|
||
| * It's simple -- a single source file implementation | ||
| * It's tested -- have confidence it works [](https://travis-ci.org/mitre/HTTP-Proxy-Servlet) | ||
| * It's tested -- have confidence it works [](https://travis-ci.org/madhbhavikar/HTTP-Proxy-Servlet) |
There was a problem hiding this comment.
it was local change, for my company, will revert. and repush
| <connection>scm:git:https://dsmiley@github.com/dsmiley/HTTP-Proxy-Servlet.git</connection> | ||
| <developerConnection>scm:git:git@github.com:dsmiley/HTTP-Proxy-Servlet.git</developerConnection> | ||
| <tag>HEAD</tag> | ||
| <tag>smiley-http-proxy-servlet-1.9</tag> |
There was a problem hiding this comment.
hmm; I'm not sure what the implication of this change is... but I see it's something to maintain between versions :-/
| <!-- I used to work for MITRE for many years but I don't anymore. --> | ||
| <!--<organization>MITRE</organization>--> | ||
| </developer> | ||
| <developer> |
There was a problem hiding this comment.
I think there's a <role>contributor</role> or something like that which is supported. Is the presence of your name here a big deal here for you? I can think of a contributor or two that contributed substantially more than this PR (which doesn't mean I'm not grateful for all contributions!). Maybe me finally creating a CHANGES.txt would be a good thing to appease the desire to get credit beyond git history. It'd list names.
| @@ -131,7 +137,44 @@ protected HttpHost getTargetHost(HttpServletRequest servletRequest) { | |||
| * it can be overridden. | |||
There was a problem hiding this comment.
I think these javadocs are now missing info about the ${} syntax you added and properties files.
There was a problem hiding this comment.
Java docs added to the method
| * if defined for this proxy URL. | ||
| */ | ||
| protected static final Pattern TEMPLATE_PATTERN = Pattern.compile("\\{([a-zA-Z0-9_%.]+)\\}"); | ||
| protected static final Pattern TEMPLATE_PATTERN = Pattern.compile("\\{([a-zA-Z0-9_%-.]+)\\}"); |
There was a problem hiding this comment.
I was just looking at the RFC spec. It turns out most characters are supported except a small number. To keep this simple excepting all the characters we should (yet some we shouldn't admittedly), lets simplify this: Pattern.compile("\\{(.+?)\\}")
There was a problem hiding this comment.
pattern has been corrected
| params.put(pair.getName(), pair.getValue()); | ||
| } | ||
|
|
||
| LinkedHashMap<String, String> specialHeaders = getVariablesFromRequestHeaders(servletRequest); |
There was a problem hiding this comment.
Why are the headers pulled in advanced to a map here? Instead, why not simply resolve via request.getHeader(headerName)?
|
All changes have been incorporated, let me know if any changes are required. |
|
Ok; perhaps this weekend I'll take a look. I'm ridiculously behind in my day job work. |
|
That would be Great!, My company needs the official fix ;) |
|
The only thing missing is a test. |
|
Sorry to have neglected this. If you're still around and address what's missing, I can give it another look. Seems I last asked for a test. At least some test; needn't be comprehensive. |
2 participants
Support for properties file
Fix for possible memory leak in formatter
Support for properties file
Documentation updated
version bumped to 1.9.1
Reference : Previous pull request