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

TT-10701, fix for urlRewrite encoding issue #5835

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

andrei-tyk
Copy link
Contributor

Description

Adapted logic such that the middleware is run only once but with the caveat that if no matches are found using the urlEncoded path, another regex match check is done but this time with the path decoded. This should allow for backwards compatibility and even improve performance.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

… with the caveat that if no matches are found using the urlEncoded path, another regex match check is done but this time with the path decoded
Copy link
Contributor

github-actions bot commented Dec 5, 2023

PR Analysis

  • 🎯 Main theme: Fixing URL encoding issue in URL rewrite middleware
  • 📝 PR summary: This PR addresses a bug in the URL rewrite middleware where the middleware was not handling URL encoded paths correctly. The changes ensure that the middleware is run only once and if no matches are found using the URL encoded path, another regex match check is done but this time with the path decoded.
  • 📌 Type of PR: Bug fix
  • 🧪 Relevant tests added: No
  • ⏱️ Estimated effort to review [1-5]: 3, because the PR involves changes in the core logic of URL rewriting which requires a good understanding of the existing codebase and the issue at hand. The changes are not too extensive but are critical.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR is well-structured and addresses a specific issue. However, it would be beneficial to include tests that specifically cover the changes made. This would help in ensuring the robustness of the solution and prevent potential regressions in the future.

  • 🤖 Code feedback:
    • relevant file: gateway/mw_url_rewrite.go
      suggestion: Consider handling the error returned by url.PathUnescape function. Currently, the error is ignored which might lead to unexpected behavior if the URL path cannot be unescaped. [important]
      relevant line: unescapedPath, err := url.PathUnescape(rawPath)

    • relevant file: gateway/mw_url_rewrite.go
      suggestion: It would be good to add a comment explaining why the check for escaped characters is necessary and why the URL path is unescaped if it contains escaped characters. This would help in understanding the logic and the reason behind these checks. [medium]
      relevant line: if len(matchGroups) == 0 && containsEscapedChars(rawPath) {

How to use

Instructions

To invoke the PR-Agent, add a comment using one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

Copy link

sweep-ai bot commented Dec 5, 2023

Apply Sweep Rules to your PR?

  • Apply: All new business logic should have corresponding unit tests.
  • Apply: Refactor large functions to be more modular.
  • Apply: Add docstrings to all functions and file headers.

Copy link
Contributor

github-actions bot commented Dec 5, 2023

API Changes

no api changes detected

@buger
Copy link
Member

buger commented Dec 5, 2023

API tests result: success
Branch used: refs/pull/5835/merge
Commit:
Triggered by: pull_request (@andrei-tyk)
Execution page

@buger
Copy link
Member

buger commented Dec 5, 2023

API tests result - postgres15-sha256 env: success
Branch used: refs/heads/master
Commit: 8973d31 TT-10701, fix for urlRewrite encoding issue (#5835)

Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to change)
  • Refactoring or add test (improvements in base code or adds test
    coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning
    why it's required
  • I would like a code coverage CI quality gate exception and have
    explained why
    Triggered by: push (@andrei-tyk)
    Execution page

@buger
Copy link
Member

buger commented Dec 5, 2023

API tests result - mongo44-sha256 env: success
Branch used: refs/heads/master
Commit: 8973d31 TT-10701, fix for urlRewrite encoding issue (#5835)

Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to change)
  • Refactoring or add test (improvements in base code or adds test
    coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning
    why it's required
  • I would like a code coverage CI quality gate exception and have
    explained why
    Triggered by: push (@andrei-tyk)
    Execution page

@buger
Copy link
Member

buger commented Dec 5, 2023

API tests result - postgres15-murmur64 env: success
Branch used: refs/heads/master
Commit: 8973d31 TT-10701, fix for urlRewrite encoding issue (#5835)

Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to change)
  • Refactoring or add test (improvements in base code or adds test
    coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning
    why it's required
  • I would like a code coverage CI quality gate exception and have
    explained why
    Triggered by: push (@andrei-tyk)
    Execution page

@buger
Copy link
Member

buger commented Dec 5, 2023

API tests result - mongo44-murmur64 env: success
Branch used: refs/heads/master
Commit: 8973d31 TT-10701, fix for urlRewrite encoding issue (#5835)

Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to change)
  • Refactoring or add test (improvements in base code or adds test
    coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning
    why it's required
  • I would like a code coverage CI quality gate exception and have
    explained why
    Triggered by: push (@andrei-tyk)
    Execution page

… issues with code maintainability due to missing error checks for benchmarks
@buger
Copy link
Member

buger commented Dec 5, 2023

API tests result: success
Branch used: refs/pull/5835/merge
Commit: 3c11136
Triggered by: pull_request (@andrei-tyk)
Execution page

"OneVal Special Case",
"test/val/(.*)", "/test/val/$1",
"/test/val/VALUE%2C", "/test/val/VALUE%2C",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to extend with a test re: a query param? r.URL.String is used on L42, rather than .Path and we don't assert on query param values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the required test

Copy link

sonarcloud bot commented Dec 6, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell D 1 Code Smell

83.3% 83.3% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@buger
Copy link
Member

buger commented Dec 6, 2023

API tests result: success
Branch used: refs/pull/5835/merge
Commit: 4ee4ce4
Triggered by: pull_request (@andrei-tyk)
Execution page

@andrei-tyk andrei-tyk merged commit 8973d31 into master Dec 6, 2023
21 of 22 checks passed
@andrei-tyk andrei-tyk deleted the TT-10701-url-rewrite-bug-5-2-3 branch December 6, 2023 09:17
Copy link

tykbot bot commented Dec 6, 2023

@andrei-tyk Sorry, but I do not understand!

@andrei-tyk
Copy link
Contributor Author

/release to release-5-lts

Copy link

tykbot bot commented Dec 6, 2023

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Dec 6, 2023
<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit 8973d31)
Copy link

tykbot bot commented Dec 6, 2023

@andrei-tyk Succesfully merged PR

@andrei-tyk
Copy link
Contributor Author

/release to release-5.0.9

Copy link

tykbot bot commented Dec 6, 2023

Working on it! Note that it can take a few minutes.

@andrei-tyk
Copy link
Contributor Author

/release to release-5.2

@andrei-tyk
Copy link
Contributor Author

/release to release-5.2.4

Copy link

tykbot bot commented Dec 6, 2023

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Dec 6, 2023
<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit 8973d31)
Copy link

tykbot bot commented Dec 6, 2023

@andrei-tyk Succesfully merged PR

Copy link

tykbot bot commented Dec 6, 2023

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Dec 6, 2023
<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit 8973d31)
Copy link

tykbot bot commented Dec 6, 2023

@andrei-tyk Succesfully merged PR

tykbot bot pushed a commit that referenced this pull request Dec 6, 2023
<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit 8973d31)
Copy link

tykbot bot commented Dec 6, 2023

Still working...

Copy link

tykbot bot commented Dec 6, 2023

@andrei-tyk Succesfully merged PR

buger added a commit that referenced this pull request Dec 6, 2023
…#5835)

TT-10701, fix for urlRewrite encoding issue (#5835)

<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why
buger added a commit that referenced this pull request Dec 6, 2023
…5835)

TT-10701, fix for urlRewrite encoding issue (#5835)

<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why
buger added a commit that referenced this pull request Dec 6, 2023
…#5835)

TT-10701, fix for urlRewrite encoding issue (#5835)

<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why
titpetric pushed a commit that referenced this pull request Dec 7, 2023
<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit 8973d31)
buger added a commit that referenced this pull request Dec 7, 2023
…#5835)

TT-10701, fix for urlRewrite encoding issue (#5835)

<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why
@andrei-tyk
Copy link
Contributor Author

/release to release-4-lts

Copy link

tykbot bot commented Dec 7, 2023

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Dec 7, 2023
<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit 8973d31)
Copy link

tykbot bot commented Dec 7, 2023

@andrei-tyk Succesfully merged PR

buger added a commit that referenced this pull request Dec 7, 2023
…#5835)

TT-10701, fix for urlRewrite encoding issue (#5835)

<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why
lghiur pushed a commit that referenced this pull request Jan 22, 2024
…#5835)

TT-10701, fix for urlRewrite encoding issue (#5835)

<!-- Provide a general summary of your changes in the Title above -->

## Description

Adapted logic such that the middleware is run only once but with the
caveat that if no matches are found using the urlEncoded path, another
regex match check is done but this time with the path decoded. This
should allow for backwards compatibility and even improve performance.
<!-- Describe your changes in detail -->

## Related Issue

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why
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.

3 participants