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

Undertow endpoints cannot deserialize optional header params sent by Feign clients #1330

Open
pkoenig10 opened this issue Nov 20, 2019 · 4 comments

Comments

@pkoenig10
Copy link
Member

What happened?

A service is exposing a Conjure Undertow endpoint with an Optional<AuthHeader> header parameter. A Conjure Feign client calls this endpoint with an Optional.empty() value and receives a 400 because this parameter fails to deserialize.

What did you want to happen?

The request from the client should have succeeded.

Details

This is related to #790.

This request works against a Jersey service because Jersey will return null if an exception is thrown while deserializing an empty string.

@pkoenig10
Copy link
Member Author

I can provide a link to an internal CI build that demonstrates this behavior.

@carterkozak
Copy link
Contributor

Same problem as #1331 (comment)

The conjure spec requires optional headers to be absent from the request to deserialize into an empty optional, otherwise we would not be able to differentiate between a present optional of an empty string, and and absent optional.

@pkoenig10
Copy link
Member Author

Right. Ideally we would fix #790. Although it doesn't seems like we are planning to do that.

Also, without a server-side fix, servers with optional parameters cannot switch to using Undertow endpoints because they cannot guarantee that they will not break existing clients.

The only reason optional parameters work at all today is because both clients and servers do not adhere to the spec.

@a-k-g
Copy link
Contributor

a-k-g commented May 20, 2020

Just ran into this in another internal product.

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 a pull request may close this issue.

3 participants