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

Restore scheme/host when recording HTTP(S) #22102

Merged
merged 1 commit into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdk/internal/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

* Recording will restore the original scheme/host after making a successful HTTP(s) call.

### Other Changes

## 1.5.0 (2023-11-02)
Expand Down
21 changes: 15 additions & 6 deletions sdk/internal/recording/recording.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,10 +537,10 @@ func defaultOptions() *RecordingOptions {
}
}

func (r RecordingOptions) ReplaceAuthority(t *testing.T, rawReq *http.Request) *http.Request {
func (r RecordingOptions) ReplaceAuthority(t *testing.T, rawReq *http.Request) (*http.Request, string, string) {
originalURLScheme := rawReq.URL.Scheme
originalURLHost := rawReq.URL.Host
if GetRecordMode() != LiveMode && !IsLiveOnly(t) {
originalURLHost := rawReq.URL.Host

// don't modify the original request
cp := *rawReq
cpURL := *cp.URL
Expand All @@ -556,7 +556,7 @@ func (r RecordingOptions) ReplaceAuthority(t *testing.T, rawReq *http.Request) *
cp.Header.Set(IDHeader, GetRecordingId(t))
rawReq = &cp
}
return rawReq
return rawReq, originalURLScheme, originalURLHost
}

func (r RecordingOptions) host() string {
Expand Down Expand Up @@ -941,8 +941,17 @@ type RecordingHTTPClient struct {
}

func (c RecordingHTTPClient) Do(req *http.Request) (*http.Response, error) {
req = c.options.ReplaceAuthority(c.t, req)
return c.defaultClient.Do(req)
req, origScheme, origHost := c.options.ReplaceAuthority(c.t, req)
resp, err := c.defaultClient.Do(req)
if err != nil {
return nil, err
}
// if the request succeeds, restore the scheme/host with their original values.
// this is imporant for things like LROs that might use the originating URL to
// poll for status and/or fetch the final result.
resp.Request.URL.Scheme = origScheme
resp.Request.URL.Host = origHost
return resp, nil
}

// NewRecordingHTTPClient returns a type that implements `azcore.Transporter`. This will automatically route tests on the `Do` call.
Expand Down
4 changes: 2 additions & 2 deletions sdk/internal/recording/recording_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,8 @@ func (s *recordingTests) TestStartStopRecordingClient() {
require.NoError(err)
require.Equal("https://azsdkengsys.azurecr.io/acr/v1/some_registry/_tags",
data.Entries[0].RequestURI)
require.Equal(resp.Request.URL.String(),
fmt.Sprintf("%s/acr/v1/some_registry/_tags", defaultOptions().baseURL()))
require.Equal("https://azsdkengsys.azurecr.io/acr/v1/some_registry/_tags",
resp.Request.URL.String())
}

func (s *recordingTests) TestStopRecordingNoStart() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"Entries": [],
"Variables": {
"randSeed": "1689722394"
"randSeed": "1701821574"
}
}