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

Trim spaces in CORS config #12177

Merged
merged 1 commit into from
Sep 19, 2020
Merged

Conversation

stuartwdouglas
Copy link
Member

Adding spaces to the lists causes hard to debug
errors.

@knutwannheden
Copy link
Contributor

Wouldn't it make sense if trimming the the elements in a comma-separated list of values were the default behavior of Quarkus, when mapping to a List typed ConfigItem? That is what I would have expected in any case.

@sberyozkin
Copy link
Member

sberyozkin commented Sep 18, 2020

@radcortez Hi Roberto, where such a global feature can be controlled, in Quarkus or directly in smallrye-config ?
@knutwannheden good point, the PR is good as it fixes a concrete problem, but indeed, I've certainly had to deal with trimming other list properties by manual trim() :-). I think we should merge this PR and if it can be done by default in Quarkus without any side-effects then all the trim() code dealing with the configs can be gradually removed

@radcortez
Copy link
Member

@radcortez Hi Roberto, where such a global feature can be controlled, in Quarkus or directly in smallrye-config ?

SR Config :) Strange. I thought that it was already trimming spaces, but I guess not. I'll have to check. We are also going to improve how empty values are handled: eclipse/microprofile-config#446

@gsmet
Copy link
Member

gsmet commented Sep 18, 2020

IIRC it trims spaces for some types (e.g. Integer, boolean) but not for Strings.

Otherwise, it would be impossible to configure something ending with a space, which could happen (think of a prefix in the subject of emails you send).

There's no real perfect solution for this. Or maybe it could be the default and then we would have a specific annotation or parameter to say "don't trim this string".

@radcortez
Copy link
Member

Ah, yes. Trimming is only applied for non String types.

Adding spaces to the lists causes hard to debug
errors.
@gsmet
Copy link
Member

gsmet commented Sep 18, 2020

Got hit by CI master issue, I rebased and force pushed.

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 18, 2020
@gsmet gsmet added this to the 1.9.0 - master milestone Sep 18, 2020
@jaikiran jaikiran merged commit 89222cb into quarkusio:master Sep 19, 2020
@gsmet gsmet modified the milestones: 1.9.0 - master, 1.8.2.Final Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants