Merge pull request #2442 from github/redirect-301

Refuse to follow HTTP 301, 302 redirects for non-GET requests

features/pull_request.feature

@@ -1189,7 +1189,7 @@ 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
+  Scenario: Pull request with 307 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:
@@ -1214,6 +1214,36 @@ 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 301 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:
+      """
+      get('/repos/mislav/coral') {
+        redirect 'https://api.github.com/repositories/12345', 301
+      }
+      get('/repositories/12345') {
+        json :name => 'coralify', :owner => { :login => 'coral-org' }
+      }
+      post('/repos/mislav/coral/pulls') {
+        redirect 'https://api.github.com/repositories/12345/pulls', 301
+      }
+      post('/repositories/12345/pulls', :host_name => 'api.github.com') {
+        assert :base  => 'master',
+               :head  => 'coral-org:feature',
+               :title => 'hereyougo'
+        status 201
+        json :html_url => "the://url"
+      }
+      """
+    When I run `hub pull-request -m hereyougo`
+    Then the exit status should be 1
+    And stderr should contain exactly:
+      """
+      Error creating pull request: Post https://api.github.com/repositories/12345/pulls: refusing to follow HTTP 301 redirect for a POST request
+      Have your site admin use HTTP 308 for this kind of redirect
+      """
+
   Scenario: Default message with --push
     Given the git commit editor is "true"
     Given the GitHub API server:

github/http.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"crypto/md5"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -198,10 +199,35 @@ func newHttpClient(testHost string, verbose bool, unixSocket string) *http.Clien
 	}
 
 	return &http.Client{
-		Transport: tr,
+		Transport:     tr,
+		CheckRedirect: checkRedirect,
 	}
 }
 
+func checkRedirect(req *http.Request, via []*http.Request) error {
+	var recommendedCode int
+	switch req.Response.StatusCode {
+	case 301:
+		recommendedCode = 308
+	case 302:
+		recommendedCode = 307
+	}
+
+	origMethod := via[len(via)-1].Method
+	if recommendedCode != 0 && !strings.EqualFold(req.Method, origMethod) {
+		return fmt.Errorf(
+			"refusing to follow HTTP %d redirect for a %s request\n"+
+				"Have your site admin use HTTP %d for this kind of redirect",
+			req.Response.StatusCode, origMethod, recommendedCode)
+	}
+
+	// inherited from stdlib defaultCheckRedirect
+	if len(via) >= 10 {
+		return errors.New("stopped after 10 redirects")
+	}
+	return nil
+}
+
 func cloneRequest(req *http.Request) *http.Request {
 	dup := new(http.Request)
 	*dup = *req