Skip to content

Fixing responses when unexpected errors occurs#10383

Merged
stianst merged 1 commit intokeycloak:mainfrom
pedroigor:issue-10338
Feb 23, 2022
Merged

Fixing responses when unexpected errors occurs#10383
stianst merged 1 commit intokeycloak:mainfrom
pedroigor:issue-10338

Conversation

@pedroigor
Copy link
Copy Markdown
Contributor

Closes #10338

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
response.write(Buffer.buffer(""));
response.write(Buffer.buffer(""));
response.end();

Make it sense to end the requests here? I guess that should prevent any additional writes on the request.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried that and it seems unnecessary because that is done later by vert.x itself.

jkroepke
jkroepke previously approved these changes Feb 22, 2022
Copy link
Copy Markdown
Contributor

@jkroepke jkroepke left a comment

Choose a reason for hiding this comment

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

LGTM. I tested this PR with the provided shell script from #10338 + the keycloak-config-cli test suite.

The Quarkus distribution has now the same behavior as Wildfly based Keycloak.

DGuhr
DGuhr previously approved these changes Feb 22, 2022
Copy link
Copy Markdown
Contributor

@DGuhr DGuhr left a comment

Choose a reason for hiding this comment

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

LGTM too. Nice! 👍

@jkroepke I still think that in the future we should add proper request validation on the api level (see e.g. #10217 ), so "same behaviour as wildfly" is not necessarily good behaviour. But this for sure should be done in follow-up issues :)

@pedroigor
Copy link
Copy Markdown
Contributor Author

@DGuhr I'm updating the PR with a fix to the new tests to make it pass for Wildfly. Sorry, I had to dismiss your review.

@stianst stianst merged commit 209df44 into keycloak:main Feb 23, 2022
pedroigor added a commit to pedroigor/keycloak that referenced this pull request Mar 21, 2022
stianst pushed a commit that referenced this pull request Mar 21, 2022
@stianst stianst mentioned this pull request Mar 22, 2022
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.

On SQLException, invalid Content-Length header in response

4 participants