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: Fix Redirect issue in latest BAS on CF #1056

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

FrankEssenberger
Copy link
Contributor

Relates to #1037 and changes the logic that first the call is made against the URL with a / in the end.

Copy link
Contributor

@jjtang1985 jjtang1985 left a comment

Choose a reason for hiding this comment

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

Looks good.
With slash and without slashes are different URLs. Different service might treat them differently like:

  • treat them with the same behaviour by redirecting from one to another
  • treat them completely different with different behaviour

So far, all the issues come from on-promise system that we want to fix, which might not behave the same as the cloud services or other OData Services.
Therefore, we either try A then switch to B like this PR, or make a new option for the generator, so that every service can map to it's csrf token url.

I guess this is a quick win. If there are some complaints about the performance due to the additional requests, we might switch to the mapping.

@FrankEssenberger
Copy link
Contributor Author

@jjtang1985 I checked the cloud system and there both versions work with and wouthout slash. I assume that only the onPremise is broken in a sense that it likes the / in the end. But we will keep and eye on this.

@FrankEssenberger FrankEssenberger merged commit ab5826c into main Mar 5, 2021
@FrankEssenberger FrankEssenberger deleted the improve-header-logging branch March 5, 2021 14:26
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.

None yet

2 participants