As of this morning, trying to use latest bzexport on Windows I get failures with a stack that ends in: File "c:/Users/Brian/version-control-tools/hgext\bzexport\__init__.py", line 1041, in bzexport "attachment.cgi?id=" + result["id"] + "&action=edit") KeyError: 'id' Others on Linux have confirmed seeing the same behaviour. At a glance, it seems that create_attachment is now returning an object with a single property, 'attachments' that has an array of attachments. Hence, the 'id' property it is looking for is not found. This was working yesterday so seems to coincide with bug 1098342 taking effect.

After working around this particular issue, other issues crop up: addReadyPromise2 uploaded as https://bugzilla.mozilla.org/attachment.cgi?id=8528192&action=edit Error: HTTP Error 301: Moved Permanently abort: Could not update attachment 8528194 [details] [diff] [review]: No JSON object could be decoded Basically, bzexport is all sorts of fail after bug 1098342.

For reference, here are my hacks to bzexport to work around the first issue.

I just hit this on Linux.

Birtles's patch doesn't actually lead to attachment upload succeeding, though. It seems like the API described at: https://wiki.mozilla.org/Bugzilla:BzAPI:Methods#Create_new_attachment_.28.2Fbug.2F.3Cid.3E.2Fattachment_POST.29 is now broken. It: * fails to create the attachment * returns a JSON blob representing the list of current attachments on the bug, rather than the new attachment

(Moving assignee from bug 1104906)

I was able to upload the patch in bug 1033394 whilst using bugzilla.m.o/bzapi/, though given comment 4, perhaps this is just because that bug already had an attachment? (ie if we're just returning a json blob of the current attachments, then we won't get the key error if there was already one)

(In reply to Ed Morley (moved to Treeherder team) [:edmorley] from comment #8) > I was able to upload the patch in bug 1033394 whilst using > bugzilla.m.o/bzapi/, though given comment 4, perhaps this is just because > that bug already had an attachment? (ie if we're just returning a json blob > of the current attachments, then we won't get the key error if there was > already one) Try marking all the existing ones obsolete and then try again to test that hypothesis? :-)

(In reply to :Gijs Kruitbosch from comment #9) > Try marking all the existing ones obsolete and then try again to test that > hypothesis? :-) I wasn't able to prove this on bugzilla-dev.allizom.org/bzapi/. In fact, I wasn't able to repro at all using either bugzilla-dev.allizom.org/bzapi/ or bugzilla.mozilla.org/bzapi/. Do we think that it was perhaps the redirect that is causing issues? ie: if the POST was flipped to a GET (after bug 1036802 I'll believe anything lol) we would get a list of existing attachments, which would seem to explain the rest of the behaviour, and why I cannot repro.

The debugging patch I was using, in case it helps. Example request (though wasn't able to repro with this): [/c/src/vctools]$ hg bzexport saved edited form in .hg/last_bzexport.txt Requesting review from emorley@SNIP.com Request URL: https://bugzilla-dev.allizom.org/bzapi/bug/1006954/attachment?username=emorley%40SNIP.com&password=REMOVED Request headers: {'Content-Type': 'application/json', 'Accept': 'application/json'} Request body: {"description": "Foo", "encoding": "base64", "file_name": "2014-11-26_11-26-10_r1542+.diff", "is_patch": true, "comments": [{"text": "Test"}], "flags": [{"requestee": {"name": "emorley@SNIP.com"}, "status": "?", "name": "review", "type_id": 4}], "content_type": "text/plain", "data": "SNIP"} Response headers: Server: Apache X-Backend-Server: web5.stage.bugs.scl3.mozilla.com Vary: Accept-Encoding Content-Type: application/json; charset=UTF-8 Strict-transport-security: max-age=31536000; includeSubDomains Date: Wed, 26 Nov 2014 14:22:22 GMT Keep-Alive: timeout=5, max=1000 X-xss-protection: 1; mode=block Transfer-Encoding: chunked Access-control-allow-origin: * Etag: f+QLO67xq6ynOhjAr0Mz5A X-content-type-options: nosniff Connection: close Set-Cookie: Bugzilla_login=REMOVED; path=/; secure; HttpOnly Set-Cookie: Bugzilla_logincookie=REMOVED; path=/; secure; HttpOnly X-frame-options: SAMEORIGIN Access-control-allow-headers: origin, content-type, accept Response body: {"ref": "https://bugzilla-dev.allizom.org/bzapi/attachment/8418124", "id": "8418124"} 2014-11-26_11-26-10_r1542+.diff uploaded as https://bugzilla.mozilla.org/attachment.cgi?id=8418124&action=edit

Note "Request headers" in comment 11 are only what was passed to urllib2.Request() and doesn't include whatever other default headers it adds (should you wish to convert that to cURL).

The redirect traffic script wasn't changing the method, but it does look like it was stripping the query string. :-( You can still test against the redirect by adding an /etc/host entry that points api-dev.b.m.o to 63.245.215.123.

(In reply to Kendall Libby [:fubar] from comment #13) > The redirect traffic script wasn't changing the method, but it does look > like it was stripping the query string. :-( Or not? Poor documentation is poor. http.getPath strips the query string, but http.setPath says it preserves any existing query string unless the replacement contains a '?'. >.< Here's the traffic script in question, fwiw: $path = http.getPath(); if( string.startsWith( $path, "/latest/") || string.startsWith( $path, "/1.3/") || string.startsWith( $path, "/1.2/") ) { $newpath = string.regexsub( $path, "^/(latest|1.2|1.3)/", "/bzapi/" ); http.setPath( $newpath ); http.changesite( "https://bugzilla.mozilla.org" ); } else { connection.discard(); }

(In reply to Kendall Libby [:fubar] from comment #13) > You can still test against the redirect by adding an /etc/host entry that > points api-dev.b.m.o to 63.245.215.123. Yeah I can now repro when using the redirect.

(In reply to Ed Morley (moved to Treeherder team) [:edmorley] from comment #15) > (In reply to Kendall Libby [:fubar] from comment #13) > > You can still test against the redirect by adding an /etc/host entry that > > points api-dev.b.m.o to 63.245.215.123. > > Yeah I can now repro when using the redirect. Ok So we have evidence that the redirect itself is munging the request. We can see if we can get a header dump going in and after the redirect to see what is being changed. If in fact the POST is being changed to a GET somehow that would explain what is happening when the client is getting a list of attachments on the bug instead the new attachment id returned. dkl

Ok so as far as I can tell, this turns out to be due to RFC2616, or more various client's interpretations of it: http://tools.ietf.org/html/rfc2616.html#section-10.3.2 10.3.2 301 Moved Permanently ... If the 301 status code is received in response to a request other than GET or HEAD, the user agent MUST NOT automatically redirect the request unless it can be confirmed by the user, since this might change the conditions under which the request was issued. Note: When automatically redirecting a POST request after receiving a 301 status code, some existing HTTP/1.0 user agents will erroneously change it into a GET request. This affects Python's urllib2: https://docs.python.org/2/library/urllib2.html#urllib2.HTTPRedirectHandler.redirect_request "The default implementation of this method does not strictly follow RFC 2616, which says that 301 and 302 responses to POST requests must not be automatically redirected without confirmation by the user. In reality, browsers do allow automatic redirection of these responses, changing the POST to a GET, and the default implementation reproduces this behavior." So basically the spec says don't redirect if !(GET or HEAD) (which would help anyway), and browsers, cURL and urllib2 ignore the spec and redirect anyway, but change the request to a GET. Seems like we either cannot redirect legacy BMO for POST requests, or are going to have add handling on the bzapi comp side to ignore the request type and try to derive it from the content?

s/which would help/which would not help/

Hmm. I would rather not hack the code to look in the request body to determine whether it is GET or POST. What if we too twiddle DNS to just make all requests to api-dev.bugzilla.mozilla.org and the use mod_rewrite in .htaccess to rewrite the path? Example: RewriteRule ^(latest|1\.2|1\.3)/(.*)$ bzapi/$1 [NE] dkl

(In reply to David Lawrence [:dkl] from comment #19) > Hmm. I would rather not hack the code to look in the request body to > determine whether it is GET or POST. > > What if we too twiddle DNS to just make all requests to > api-dev.bugzilla.mozilla.org and the use mod_rewrite in .htaccess to rewrite > the path? > > Example: > RewriteRule ^(latest|1\.2|1\.3)/(.*)$ bzapi/$1 [NE] > > dkl This is one option - or we could make the redirect script to do actual proxying of the request (which of course could lead to serious issues if there was a problem with how it did that or what requests it made or how our firewall setup there works etc.).

(In reply to David Lawrence [:dkl] from comment #19) > Hmm. I would rather not hack the code to look in the request body to > determine whether it is GET or POST. > > What if we too twiddle DNS to just make all requests to > api-dev.bugzilla.mozilla.org and the use mod_rewrite in .htaccess to rewrite > the path? > > Example: > RewriteRule ^(latest|1\.2|1\.3)/(.*)$ bzapi/$1 [NE] Ah of course this is much simpler. Either way, I think we can close this bug out now - discussions about a non-301-redirect solution can occur in bug 1098342. I'd suggest we switch bzexport over to the new endpoint regardless, since it will reduce the risk of bug 1098342 when it finally lands - but there's already bug 1033394 filed for that (and it looks like there may be another bzapi compatibility layer bug lurking - bug 1033394 comment 7).