luarocks / luarocks-site

LuaRocks website and module host
http://luarocks.org
176 stars 36 forks source link

`luarocks upload` command is broken after domain change #50

Closed ignacio closed 9 years ago

ignacio commented 9 years ago

Hi. It seems that after a97b17970756341fd0bf9a998aa45d027329a234 uploading rocks began to fail.

D:\trunk_git\sources\nozzle>luarocks upload rockspecs\nozzle-git-1.rockspec --verbose

io.popen:       "c:\luarocks\2.2\tools\pwd" 2> NUL

os.execute:     D: & cd "D:\trunk_git\sources\nozzle" & if not exist "rockspecs\nozzle-git-1.rockspec" invalidcommandname
Results: 1
  1 (number): 0
Sending rockspecs\nozzle-git-1.rockspec ...
[GET via luasec] https://rocks.moonscript.org/api/tool_version?current=1%2e0%2e0 ...
wrong version number
[GET via luasec] /check_rockspec?package=nozzle&version=git%2d1 ...
wrong version number

Error: API returned wrong version number - /check_rockspec?package=nozzle&version=git%2d1

Note: I have LuaSec installed. After removing it, it goes back to using 'curl', which goes a little further but fails at the end.

The only way to fix the upload is to edit upload_config.lua file and change the server line to server = "https://luarocks.org"

hishamhm commented 9 years ago

@leafo, we will push a new LR release today or tomorrow with the config update, but that won't help current users. @Tieske sent a pull requset pinging @brunoos about the LuaSec issue (which affects LR users at large, not only for upload), but that PR needs updated support from @diegonehab's LuaSocket as well (so fixing those libraries and getting updated releases will probably take a while).

While we tweak things at the LR end, is it somehow possible to make the old upload URL function without redirects, so that things don't break for users of existing versions?

Tieske commented 9 years ago

I didn't investigate any further, but my LuaSec install (including the redirect fix) gave me an error about an unsafe redirect, from https to http. Which my patch doesn't allow by default (and which LuaRocks should also not allow anyway).

So some testing needs to be done here.

ignacio commented 9 years ago

Hi @leafo. I saw you commited this change but it seems that is not enough.

A permanent redirect (301) is issued, but curl, upon receiving a 301 from the server, performs a GET instead of a POST. According to curl's manual:

When curl follows a redirect and the request is not a plain GET (for example POST or PUT), it will do the following request with a GET if the HTTP response was 301, 302, or 303. If the response code was any other 3xx code, curl will re-send the following request using the same unmodified method.

So maybe issuing a temporary redirect (307) is the safe bet for now?

leafo commented 9 years ago

I removed the redirect entirely for any /api calls. Didn't get a chance to deploy (or push). Will do in a moment.

ignacio commented 9 years ago

Thanks @leafo I can confirm that uploading is working with the old url, both with luasec and curl.

leafo commented 9 years ago

Awesome, thanks for checking.