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

fix(rest): Ensure proper URL encoding + fix case issues with content type headers #2826

Merged

Conversation

johnBgood
Copy link
Contributor

Description

  • Decode the URL before encoding it, to avoid double encoding if the provided URL was already encoded (that's a lot of encoding/decoding)
  • Fix some potential issues with contentType headers cases (multipart, and formencoded).

Related issues

closes https://github.com/camunda/team-connectors/issues/840

@johnBgood johnBgood self-assigned this Jul 8, 2024
@johnBgood johnBgood requested a review from a team as a code owner July 8, 2024 09:36
@@ -71,11 +71,11 @@ public void build(ClassicRequestBuilder builder, HttpCommonRequest request) {
private HttpEntity createEntityForContentType(
ContentType contentType, Map<?, ?> body, HttpCommonRequest request) {
HttpEntity entity;
if (contentType.getMimeType().equals(MULTIPART_FORM_DATA.getMimeType())) {
if (contentType.getMimeType().equalsIgnoreCase(MULTIPART_FORM_DATA.getMimeType())) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even parsed, Apache stores the initial value (could be case inconsistent)

@johnBgood johnBgood enabled auto-merge July 8, 2024 11:29
Comment on lines 37 to 43
// We try to decode the URL first, because it might be encoded already
// which would lead to double encoding. Decoding is safe here, because it does nothing if
// the URL is not encoded.
var decodedUrl =
URLDecoder.decode(
Optional.ofNullable(request.getUrl()).orElse(""), StandardCharsets.UTF_8);
var url = new URL(decodedUrl);
Copy link
Contributor

Choose a reason for hiding this comment

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

For easier maintenance and testability, could you extract this into a static utility method?
This and the other part and simply return a string.

@johnBgood johnBgood added this pull request to the merge queue Jul 8, 2024
Merged via the queue into main with commit e07b88c Jul 8, 2024
4 checks passed
@johnBgood johnBgood deleted the 840-rest-connector-handle-spaces-in-url-path-seond-part branch July 8, 2024 12:52
github-actions bot pushed a commit that referenced this pull request Jul 8, 2024
…type headers (#2826)

* fix(rest): Ensure proper URL encoding + fix case issues with content type headers

* fix(rest): Add error logs

* fix(rest): Refactor code structure

* fix(rest): Make the encoder static

* fix(rest): Fix license

(cherry picked from commit e07b88c)
Copy link
Contributor

github-actions bot commented Jul 8, 2024

johnBgood added a commit that referenced this pull request Jul 8, 2024
…type headers (#2826) (#2827)

* fix(rest): Ensure proper URL encoding + fix case issues with content type headers

(cherry picked from commit e07b88c)

Co-authored-by: Jonathan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants