pushing gem fails if there are redirects #88

Closed
docwhat opened this Issue Jan 3, 2013 · 5 comments

Comments

Projects
None yet
4 participants
@docwhat
Contributor

docwhat commented Jan 3, 2013

We moved our gem server from http://example.com/gems/ to https://gems.example.com/ and setup 301 redirects to so that old gem setups would continue to work.

However, doing gem inabox foobar-1.2.3.gem fails because it won't follow the 301 redirect from http://example.com/gems/ to https://gems.example.com/.

I expected it to work, like gem pull continued to work with the old URL.

I'm using version 0.8.0

@pipt

This comment has been minimized.

Show comment
Hide comment
@pipt

pipt Feb 5, 2013

Contributor

I can reproduce this with the latest version of geminabox - the problem is that HTTPClient doesn't automatically follow redirects, so the client errors when the status returned is 301:

def push(gemfile)
  response = http_client.post(url_for(:upload), {'file' => File.open(gemfile, "rb")}, {'Accept' => 'text/plain'})

  if response.status < 300
    response.body
  else
    raise GeminaboxClient::Error, "Error (#{response.code} received)\n\n#{response.body}"
  end
end

HTTPClient supports following redirects, by adding :follow_redirect => true to gets or posts, but when I tried that it just hung, so I need to do more investigation...

Contributor

pipt commented Feb 5, 2013

I can reproduce this with the latest version of geminabox - the problem is that HTTPClient doesn't automatically follow redirects, so the client errors when the status returned is 301:

def push(gemfile)
  response = http_client.post(url_for(:upload), {'file' => File.open(gemfile, "rb")}, {'Accept' => 'text/plain'})

  if response.status < 300
    response.body
  else
    raise GeminaboxClient::Error, "Error (#{response.code} received)\n\n#{response.body}"
  end
end

HTTPClient supports following redirects, by adding :follow_redirect => true to gets or posts, but when I tried that it just hung, so I need to do more investigation...

@pipt

This comment has been minimized.

Show comment
Hide comment
@pipt

pipt Feb 6, 2013

Contributor

There's a comment before the post method in HTTPClient that says the following:

# Sends POST request to the specified URL.  See request for arguments.
# You should not depend on :follow_redirect => true for POST method.  It
# sends the same POST method to the new location which is prohibited in HTTP spec.

Looking at the HTTP/1.1 spec, it prohibits user agents from following redirects without user interaction unless the method used in the second request is GET or HEAD:

10.3 Redirection 3xx
This class of status code indicates that further action needs to be taken by the user agent in order to fulfill the request. The action required MAY be carried out by the user agent without interaction with the user if and only if the method used in the second request is GET or HEAD. A client SHOULD detect infinite redirection loops, since such loops generate network traffic for each redirection.

After reading this, I don't think the geminabox client should be following the redirect when uploading a gem, so I'm closing the issue.

A possible workaround for your use case would be to proxy from http://example.com/gems to http://gems.example.com instead of using a 301.

Contributor

pipt commented Feb 6, 2013

There's a comment before the post method in HTTPClient that says the following:

# Sends POST request to the specified URL.  See request for arguments.
# You should not depend on :follow_redirect => true for POST method.  It
# sends the same POST method to the new location which is prohibited in HTTP spec.

Looking at the HTTP/1.1 spec, it prohibits user agents from following redirects without user interaction unless the method used in the second request is GET or HEAD:

10.3 Redirection 3xx
This class of status code indicates that further action needs to be taken by the user agent in order to fulfill the request. The action required MAY be carried out by the user agent without interaction with the user if and only if the method used in the second request is GET or HEAD. A client SHOULD detect infinite redirection loops, since such loops generate network traffic for each redirection.

After reading this, I don't think the geminabox client should be following the redirect when uploading a gem, so I'm closing the issue.

A possible workaround for your use case would be to proxy from http://example.com/gems to http://gems.example.com instead of using a 301.

@pipt pipt closed this Feb 6, 2013

@docwhat

This comment has been minimized.

Show comment
Hide comment
@docwhat

docwhat Feb 11, 2013

Contributor

Or you could detect it and raise an error with a message like:

The url http://example.com/gems redirected to http://gems.example.com/ 
If this is correct, please update your .gemrc with the new URL.

Ciao!

Contributor

docwhat commented Feb 11, 2013

Or you could detect it and raise an error with a message like:

The url http://example.com/gems redirected to http://gems.example.com/ 
If this is correct, please update your .gemrc with the new URL.

Ciao!

@pipt

This comment has been minimized.

Show comment
Hide comment
@pipt

pipt Feb 11, 2013

Contributor

You're right, we could be much more user friendly here. I'll reopen the issue and add a better error message.

Thanks!

Contributor

pipt commented Feb 11, 2013

You're right, we could be much more user friendly here. I'll reopen the issue and add a better error message.

Thanks!

@pipt pipt reopened this Feb 11, 2013

@mikegee

This comment has been minimized.

Show comment
Hide comment
@mikegee

mikegee Jun 22, 2014

POSTing again to the redirection target is valid behavior after a 307 redirect.

mikegee commented Jun 22, 2014

POSTing again to the redirection target is valid behavior after a 307 redirect.

@tomlea tomlea added the Stale label Feb 15, 2016

@tomlea tomlea closed this Feb 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment