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

[Swift5][URLSession] Fix handling of customHeaders between retries #7527

Merged
merged 2 commits into from
Sep 28, 2020

Conversation

lightman73
Copy link
Contributor

@lightman73 lightman73 commented Sep 28, 2020

While trying to implement @4brunu suggestion for token refresh ( #5462 (comment) ), with an access token configured in customHeaders with key "Authorization", I noticed that after changing the value in the customHeaders in the TokenRefresher, and then calling shouldRetry(true), the following call would still use the old access token value.
Looking into the issue I noticed that the new value (set via MyAPI.customHeaders.updateValue(accessToken, forKey: "Authorization") before calling shouldRetry(true) was being overwritten with the old one stored in self.headers in the createURLRequest function defined in URLSessionImplementations.swift.
The proposed change creates the new headers for the modifiedRequest by adding the customHeaders last, so that they can overwrite the keys stored in self.headers, thus fixing the problem.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
  • Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@jgavris @ehyche @Edubits @jaz-ah @4brunu

Problem: custom headers could be overwritten between 
request retries following, e.g. a token refresh ( for example,
while implementing @4brunu refresh code :
OpenAPITools#5462 (comment) ).
A simple reordering of the modifiedRequest headers construction
solves the problem.
@4brunu
Copy link
Contributor

4brunu commented Sep 28, 2020

Looks good to me 👍

@wing328
Copy link
Member

wing328 commented Sep 28, 2020

@lightman73 thanks for the PR. Can you merge the latest master of the official repo into your branch and update the Swift petstore samples as well?

@lightman73
Copy link
Contributor Author

@lightman73 thanks for the PR. Can you merge the latest master of the official repo into your branch and update the Swift petstore samples as well?

Done ! I fixed all the URLSessionImplementations.swift in the PetStore samples to reflect the change in the mustache.

Just let me know if there is anything else that needs to be done :)

@wing328 wing328 added this to the 5.0.0 milestone Sep 28, 2020
@wing328 wing328 merged commit 785cbbb into OpenAPITools:master Sep 28, 2020
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.

None yet

4 participants