drupal_http_request() returns headers with the keys all lowercased. Feeds doesn't realize this, so the etag, and last-modified headers are never found.

CommentFileSizeAuthor
#10 feeds-drupal_http_request-1372074-10.patch7.09 KBemackn
#8 feeds-drupal_http_request-1372074-8.patch5.49 KBemackn
#5 feeds-drupal_http_request-1372074-5.patch2.31 KBtwistor
#4 feeds-drupal_http_request-1372074-4.patch2.02 KBtwistor
#2 feeds-drupal_http_request-1372074-2-6x.patch1.42 KBtwistor
#1 feeds-drupal_http_request-1372074-1.patch1.44 KBtwistor

Comments

twistor’s picture

twistor commented
Status: Active » Needs review
StatusFileSize
new feeds-drupal_http_request-1372074-1.patch1.44 KB

Here's the patch. I removed the weird header-setting thing at the end since the headers are checked with empty() which is fine for unset values.

Needs 6.x as well.

twistor’s picture

twistor commented
StatusFileSize
new feeds-drupal_http_request-1372074-2-6x.patch1.42 KB

6.x version.

emackn’s picture

emackn commented

drupal_http_request is normalizing all the headers to lower case. Feeds should expect this. So the fix is to update what headers are getting checked earlier on in http_request_get and updating all the header keys to be correctly normalized to match what drupal_http_request returns.

twistor’s picture

twistor commented
StatusFileSize
new feeds-drupal_http_request-1372074-4.patch2.02 KB

The other part to this is that curl doesn't normalize the headers.

twistor’s picture

twistor commented
StatusFileSize
new feeds-drupal_http_request-1372074-5.patch2.31 KB

This gets rid of the $has_etag which is never used for anything.

emackn’s picture

emackn commented

is $has_etag being used anywhere? I'm also trying to build a test for this.

twistor’s picture

twistor commented

I did a find all in http_request.inc and the only two places it's used are the two assignments that I removed. I'm guessing that it was going to be used to conditionally add the header array to the request, but latter discovered that it's unnecessary.

emackn’s picture

emackn commented
StatusFileSize
new feeds-drupal_http_request-1372074-8.patch5.49 KB

how does this work? I added an update for Content-Type on line 59 and converted the headers to lower case before they go into cache around 186.

twistor’s picture

twistor commented

Good catch on the content-type. Are there any other places that access headers? I did some searching and didn't find anything.

A couple of pedantic remarks:

  • Comments should be complete sentences, capitalized and ending with a period.
  • The file does not contain a new-line at the end.

Seems to be working well.

emackn’s picture

emackn commented
StatusFileSize
new feeds-drupal_http_request-1372074-10.patch7.09 KB

re-rolled with suggestions.

twistor’s picture

twistor commented
Status: Needs review » Reviewed & tested by the community

bang

emackn’s picture

emackn commented
Status: Reviewed & tested by the community » Fixed

committed

twistor’s picture

twistor commented
Status: Fixed » Patch (to be ported)
twistor’s picture

twistor commented
Version: 7.x-2.x-dev » 6.x-1.x-dev
twistor’s picture

twistor commented
Status: Patch (to be ported) » Fixed

6.x http://drupalcode.org/project/feeds.git/commit/73a38b5

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.