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

Ability to remove certain header fields before saving a request #25

Open
nunogoncalves opened this issue Jun 25, 2019 · 2 comments
Open

Comments

@nunogoncalves
Copy link

nunogoncalves commented Jun 25, 2019

Hi again.
We have another use case that the library doesn't support and would like to discuss if this is something that makes sense to you.

We don't want to store sensitive information in the recordings of our requests (these files are saved in the repository), such as Authorization headers. To make it completely automatic it would be useful to give the client the possibility to choose what to save in the file.
Since headers are not used to create the hash representation, changing headers doesn't really affect the reading of a request from a file.

We can't add that to the normalizedRequest because that request will also be used to make actual calls when recording is turned on.

  • Do you think this is something you'd like to support?
  • If so, I'd like to ask you what you think about this:

I have basic working solution for this issue, and would like to ask what you think about it.

In the MockDuckDelegate I added a method:
func updateBeforeSaving(_ request: URLRequest) -> URLRequest

Now in the record method:

func record(requestResponse: MockRequestResponse) {
        ...
        if let delegate = MockDuck.delegate {
            requestResponse.request = delegate.updateBeforeSaving(requestResponse.request)
        }
        ....
}

This way I can do whatever I want to my request before it is saved.

Potential Issues:

  • If the client changes components that are used for hashing, will fail to get recorded requests.
  • This would be a breaking change because the protocol would add a new method.

Let me know what you think.
Thanks

@scelis
Copy link
Contributor

scelis commented Jun 28, 2019

With my changes in #26 , the normalized request is never actually executed. It is only used for determining the location of the request on disk. Does that change this situation at all? I think the first potential issue you mention would not actually be an issue after this PR is merged because it would not affect the location of the file on disk (the changes would be made after normalizedRequest is called).

If this is all true, then I would be perfectly ok with a new delegate method that allows the developer to strip out sensitive information from the request.

@nunogoncalves
Copy link
Author

nunogoncalves commented Jun 28, 2019

The request that is saved to the disk is the one inside MockRequestResponse
which is the original request. That request is being saved here

So as it is, we don't actually have a way to remove that information.

We need to keep the request to actually hit the server when recording and saving it differently (headers). Since headers aren't used for hashing, it would "only" be an api breaking change, and the files hashes would be the same... We could even try to add a default implementation that would require no updates from the client. Would have to think about that.

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

No branches or pull requests

2 participants