Skip to content

Commit

Permalink
Follow 307 redirects in simpleApi, even for POST
Browse files Browse the repository at this point in the history
HTTP client used to automatically follow GET redirects, or turn certain
POST redirects to GET when applicable, but for opening a new pull
request for a renamed repo, we need to repeat the POST against the
updated location.
  • Loading branch information
mislav committed Aug 9, 2016
1 parent 56fce4b commit 80606a5
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 6 deletions.
34 changes: 34 additions & 0 deletions features/pull_request.feature
Original file line number Diff line number Diff line change
Expand Up @@ -659,3 +659,37 @@ Feature: hub pull-request
"""
When I successfully run `hub pull-request -m hereyougo`
Then the output should contain exactly "the://url\n"

Scenario: Pull request with redirect
Given the "origin" remote has url "https://github.com/mislav/coral.git"
And I am on the "feature" branch pushed to "origin/feature"
Given the GitHub API server:
"""
post('/repos/mislav/coral/pulls') {
redirect 'https://api.github.com/repositories/12345', 307
}
post('/repositories/12345', :host_name => 'api.github.com') {
assert :base => 'master',
:head => 'mislav:feature',
:title => 'hereyougo'
json :html_url => "the://url"
}
"""
When I successfully run `hub pull-request -m hereyougo`
Then the output should contain exactly "the://url\n"

Scenario: Redirect to another host is not followed
Given the "origin" remote has url "https://github.com/mislav/coral.git"
And I am on the "feature" branch pushed to "origin/feature"
Given the GitHub API server:
"""
post('/repos/mislav/coral/pulls') {
redirect 'https://disney.com/mouse', 307
}
"""
When I run `hub pull-request -m hereyougo`
Then the stderr should contain exactly:
"""
Error creating pull request: Temporary Redirect (HTTP 307)
Refused to follow redirect to https://disney.com/mouse\n
"""
5 changes: 5 additions & 0 deletions github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,11 @@ func FormatError(action string, err error) (ee error) {
}
}

redirectLocation := e.Response.Header.Get("Location")
if statusCode >= 300 && statusCode < 400 && redirectLocation != "" {
errorSentences = append(errorSentences, fmt.Sprintf("Refused to follow redirect to %s", redirectLocation))
}

var errorMessage string
if len(errorSentences) > 0 {
errorMessage = strings.Join(errorSentences, "\n")
Expand Down
31 changes: 25 additions & 6 deletions github/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,17 @@ type simpleClient struct {
accessToken string
}

func (c *simpleClient) performRequest(method, path string, body io.Reader, configure func(*http.Request)) (res *simpleResponse, err error) {
func (c *simpleClient) performRequest(method, path string, body io.Reader, configure func(*http.Request)) (*simpleResponse, error) {
url, err := url.Parse(path)
if err != nil {
return
if err == nil {
url = c.rootUrl.ResolveReference(url)
return c.performRequestUrl(method, url, body, configure, 2)
} else {
return nil, err
}
}

url = c.rootUrl.ResolveReference(url)
func (c *simpleClient) performRequestUrl(method string, url *url.URL, body io.Reader, configure func(*http.Request), redirectsRemaining int) (res *simpleResponse, err error) {
req, err := http.NewRequest(method, url.String(), body)
if err != nil {
return
Expand All @@ -223,9 +227,24 @@ func (c *simpleClient) performRequest(method, path string, body io.Reader, confi
configure(req)
}

var bodyBackup io.ReadWriter
if req.Body != nil {
bodyBackup = &bytes.Buffer{}
req.Body = ioutil.NopCloser(io.TeeReader(req.Body, bodyBackup))
}

httpResponse, err := c.httpClient.Do(req)
if err == nil {
res = &simpleResponse{httpResponse}
if err != nil {
return
}

res = &simpleResponse{httpResponse}
if res.StatusCode == 307 && redirectsRemaining > 0 {
url, err = url.Parse(res.Header.Get("Location"))
if err != nil || url.Host != req.URL.Host || url.Scheme != req.URL.Scheme {
return
}
res, err = c.performRequestUrl(method, url, bodyBackup, configure, redirectsRemaining-1)
}

return
Expand Down

0 comments on commit 80606a5

Please sign in to comment.