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

HttpHeaderMap forces lower-case #11

Closed
tsneed290 opened this issue Oct 16, 2018 · 3 comments
Closed

HttpHeaderMap forces lower-case #11

tsneed290 opened this issue Oct 16, 2018 · 3 comments
Assignees

Comments

@tsneed290
Copy link

Hello! Been using your as2-lib library for sending AS2 messages and I'm loving it so far!

I had a question (or potential bug) about the HttpHeaderMap.getUnifiedName() method. It seems to be forcing lower-case when adding header values to the map.

While I agree that the HTTP spec says that header names are case-insensitive, I am dealing with a recipient (FDA ESG) that is case-sensitive with respect to header names. This forced me to modify ph-commons so that header names are not lower-cased prior to injection into the header map.

I'd like to propose that HttpHeaderMap is changed so that header names are not forcefully lower-cased when inserted into the map, while the lookups remain case-insensitive. I'd like to know your thoughts on this proposal; I wouldn't mind making a contribution for this change.

Thanks,
Tim

@phax phax self-assigned this Oct 16, 2018
@phax
Copy link
Owner

phax commented Oct 16, 2018

Point taken. Lowercasing by insertion was the most reasonable choice at the time of writing, because than the lowercasing needs to happen only once and not on every compare (equalsIgnoreCase) but if it makes problems in practice, I will of course modify it....

phax added a commit that referenced this issue Oct 16, 2018
@phax
Copy link
Owner

phax commented Oct 16, 2018

Fixed in 9.1.8.
I'm currently working myself through the AS2 4.2.0 release which contains some minor things I want to fix before releasing it. This will be one of it. So thanks for the heads up.

@tsneed290
Copy link
Author

Thank you @phax!

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

2 participants