Skip to content

Conversation

romain-grecourt
Copy link
Contributor

@romain-grecourt romain-grecourt commented Jan 9, 2019

Fixes #211

Copy link
Member

@spericas spericas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be simpler for our media type parser to interpret "" as "/*" instead of matching an exact (and bogus) header?

@romain-grecourt
Copy link
Contributor Author

@spericas The initial idea is to hardcode a workaround specific to HTTPURLConnection and avoid allowing such bogus headers (*; q=.2) in general.

I'd be fine either way, what do others think ? @tomas-langer @ljnelson @barchetta

Copy link
Member

@barchetta barchetta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I view this fix as a work-around for the specific JDK bug. So I'm OK with the narrow fix as proposed.

If we later learn of other clients that violate the spec and use a single *, then it would make sense to follow Postel's Law and update the fix to be more liberal in what we accept.

@spericas
Copy link
Member

spericas commented Jan 9, 2019

@spericas The initial idea is to hardcode a workaround specific to HTTPURLConnection and avoid allowing such bogus headers (*; q=.2) in general.

Just to be clear, it wouldn't be *; q=.2 but simply* regardless of whether there are params like q or not. I just find the addition of the conditional,

acceptValues.size() == 1 && HUC_ACCEPT_DEFAULT.equals(acceptValues.get(0))

a less elegant and more fragile solution. OTOH, it may be sufficient.

@romain-grecourt
Copy link
Contributor Author

@spericas Yup, sorry I was misleading with "*; q=;.2", I did that to highlight the part of the header sent by HUC that is bogus. I agree that the current solution is not elegant and is fragile, however the main goal is to workaround that specific JDK bug.

@romain-grecourt romain-grecourt merged commit c4f90cb into master Jan 9, 2019
@barchetta barchetta deleted the helidon-211 branch January 12, 2019 03:54
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.

5 participants