requests.delete not actually deleting

[zhuffman]

A little bit of background: My name is Zach Huffman, I'm a software developer at AppFirst, Inc. We do systems monitoring and we have a suite of automated tests written in Python that we run our product through before new releases. One of the primary forms our clients' data takes is a log file, which can be remotely monitored from our website. In order to do this, the client has to go to our website (on their account, which is associated with their machine[s]) and essentially specify the path of the file that's being written to--this is called a log source. One log source per log file.

As of now, the log portion of the tests consists of creating a log file, creating a corresponding log source (via our APIs), writing to the log file, ensuring the data is being sent to our site, then deleting the log file and subsequently the log source--again, via our APIs. We use requests to facilitate this process. We create the log source with requests.post and delete it using requests.delete--or at least, we try to. We receive a response of 200, so it seems that neither of us is running into any errors, however, when we check for the presence of the log source that we supposedly just deleted, it's still there.

So, why do I think it's an issue with requests rather than with our tests? Great question. I'm actually working on these for the first time in several months, almost a year. The last time anyone was writing these tests, they were using requests 0.13.3. Currently we're running 2.0.1 (or whatever the current version is--we're installing requests via easy_install). We reference the log source by its unique id, and like I said, making that call with the current version of requests runs smoothly but fails to actually delete anything. BUT when I run the EXACT SAME (sorry for the caps, how do you italicize in this form?) COMMAND from an older VM running requests 0.13.3, the log source gets deleted.

TL;DR, I'm almost certain something happened to requests.delete between 0.13.3 and 2.0.1 that means it doesn't actually delete anymore. I'm looking into it myself right now and will, of course, post anything significant I find, but just giving you guys a heads-up.

Thanks,
Zach

[Lukasa]

Thanks for raising this issue!

It genuinely looks fine to me, testing using httpbin. My suggestion is that you use Wireshark to compare the two requests and see what's changed.

[canibanoglu]

Have you tried comparing the two Request objects to see if there's any difference between the two? You could also try checking out what the actual response content is with r.text, assuming that r is an instance of Response. The response code can be a 200 but the actual content might reveal an error message delivered as an html file.

Edit: Aaaand Lukasa beat me to it. :)

[sigmavirus24]

If your service is performing a redirect then that is likely the cause. I think we change the verb after the initial request. You should try with allow_redirects=False to see what the response is.

Also when you say "he current version of requests runs smoothly" what do you mean by "smoothy"? What does the response look like?

[Lukasa]

Once again @sigmavirus24 is right, that's almost certainly the problem.

[sigmavirus24]

@zhuffman if we don't hear back from you soon, I'm going to close this. We have no way of reliably testing this so until you give us more details it is essentially invalid.

[zhuffman]

Hi all, I apologize for keeping everyone waiting, and appreciate all the quick and thoughtful responses.

Indeed, @sigmavirus24 was correct--our service was performing a redirect, and requests handles those differently in v0.13.3 and v2.0.1. Making the requests with allow_redirects=False yielded a 301: moved permanently, so that was eye-opening. It turns out that in these tests we were using an old version of our api, so the redirect is, as I understand, the solution we implemented for the sake of backwards compatibility with older versions of the apis. The older version of requests will allow a redirect when it receives a 301 as long as a) allow_redirects is True (which it is by default--in both versions) and b) the method was not a POST [v 0.13.3, requests/modules.py, line 260]. In the new version, the same is true with the allow_redirects boolean but the method must be either a GET or a HEAD--and of course, delete is neither of those [v 2.0.2, requests/sessions.py, line 116]. Thus, exactly as @sigmavirus24 thought, DELETE gets changed to GET.

I actually finally managed to sit down with the last guy that worked on apis and he postulated that I should be able to avoid a redirect by explicitly specifying the most recent version of our apis in the tests, so that's the next step for me. But, if I may ask, why the change? Is the method change to GET a safety measure that we just so happen to not need because we've successfully setup a redirect due to the changing apis? Had we not implemented that, would the older version of requests not be functioning properly? On the same note, I'm curious about the comments above each line:

v 0.13.3: # Do what the browsers do if strict_mode is off...
v 2.0.1: # Do what browsers do, despite standards...

Thanks!

[Lukasa]

The comments indicate the exact rationale. The standard specifies that a 301 (and a 302) in response to something that isn't a GET (or a HEAD) should be followed by the same method, but only after confirming with the user. However, web browsers have basically never done that, instead following a POST with a GET if they receive a redirect.

What happened in 2.X to explain the comments is that we gave up on strict_mode. I'm not 100% on the changing DELETE to GET for 301s though. I quite want to see what @sigmavirus24 thinks.

[sigmavirus24]

I'm not either, but I thought the spec specified this. (I made some of these changes.) I don't have the time tonight to research it, but I'll tag this issue for this weekend.

[Lukasa]

I thought so to, but I reread it. The relevant sections:

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.

302 Found
If the 302 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: RFC 1945 and RFC 2068 specify that the client is not allowed to change the method on the redirected request. However, most existing user agent implementations treat 302 as if it were a 303 response, performing a GET on the Location field-value regardless of the original request method. The status codes 303 and 307 have been added for servers that wish to make unambiguously clear which kind of reaction is expected of the client.

Based on that, plus the expected purpose of 301 (never contact this URI again, always go to the moved one), suggests that maybe the same verb should be maintained.

[sigmavirus24]

I'm probably thinking of 307/308 then. I'm 👍 changing this back and might work on it tonight unless someone else wants to pick it up.

[Lukasa]

Ok, I think this is something we should do, but it's a backward incompatible change. Makes it a pain to implement in a point release. I'll tentatively schedule for 3.X (if that day ever comes).

[sigmavirus24]

@Lukasa I guess it is backwards incompatible but this behaviour is extraordinarily wrong. I doubt there is anyone actually relying on this behaviour so we should be safe fixing it.

[Lukasa]

Mmm, I'm less convinced. I certainly don't want to slip this into a bugfix release, it'd definitely have to go into 2.1. I bet there are people working around this behaviour.

[sigmavirus24]

Working around it means that if we fix it we'll improve their lives sooner rather than later. And I should have been more explicit that I didn't imagine it in anything other than 2.1.

[Lukasa]

Ok, let's call this a 2.1 bug then.

[sigmavirus24]

👍 If it isn't already clear, I view Semantic Versioning like I view PEP-8. Conform as much as is possible and sensible, but when there are grey lines that can benefit the user (or code) do it.

[Lukasa]

Ok, I've done some digging here, and it turns out that the HTTPbis are on the case. From their investigation, the state of play on a 301 two years ago was as follows:

• Safari: 301 was rewritten to GET.
• Firefox: 301 was rewritten to GET.
• Chrome: 301 was rewritten to GET.
• Opera: 301 was rewritten to GET.
• Internet Explorer: 301 POST was rewritten to GET, otherwise preserved (FFS IE, get your shit together).

This has since changed: Chrome and Firefox now align with IE (sigh), and only rewrite POST to GET. Not sure about Safari, but it's reasonable to assume that it's playing along with Chrome and Firefox. Given that this is what everyone else is doing, it's what we shall do as well. For future reference, a useful StackOverflow question is here.

[sigmavirus24]

@Lukasa just to be clear, when you say "301 was rewritten to GET", do you mean all verbs that receive a 301 are rewritten to a GET? That's the only explanation that makes sense to me given how IE behaves. Thanks for digging in and getting the right answer here.

[Lukasa]

Yeah, that's what I mean. =) So, on Safari, * -> 301 -> GET, whereas on IE POST -> 301 -> GET but DELETE -> 301 -> DELETE.