Support from Acquia helps fund testing for Drupal Acquia logo

Comments

komlenic’s picture

In limited testing these patches seem to work.

komlenic’s picture

Status: Active » Needs review
Pasqualle’s picture

Priority: Normal » Critical

The urls in the patch are correct.

But I am not sure why we are not using the PHP version of browscap.ini. It has double quotes on values, which is required for parse_ini_file

Note:
If a value in the ini file contains any non-alphanumeric characters it needs to be enclosed in double-quotes (").

mstrelan’s picture

Title: Can't dowload last Browscap.ini RC from http://http://browscap.org/ » Can't dowload last Browscap.ini RC from http://browscap.org/
FileSize
1 KB
1 KB
1 KB

Attached patches use PHP version of the ini files.

CharlesNepote’s picture

"This site and related update URLs will be disabled on 30th March 2014." says http://tempdownloads.browserscap.com/

Any chance to make a release before this date? or we have to apply the patch by hand?

Many thanks.

steveoriol’s picture

I had this message : "Couldn't check version: Gone" when I clic on the button "Refresh browscap data"
It is OK with the patch on D7.

ADrupalUser’s picture

After applying the patch on D7, attempting to Refresh Browscap Data takes a while then takes me to this:

Page Unavailable

The page you requested is temporarily unavailable.

We're redirecting you to the homepage in 5 seconds.

(Error 503 Service Unavailable)

kevster’s picture

Thx - I can confirm D7 patch worked for me after getting "gone" msg...

aharown07’s picture

Tested D7 patch on two sites. No problems to report.

WebWalker3D’s picture

I also installed the patch for D7 on #4, and get a 503 error. Any insight?

robertgarrigos’s picture

Status: Needs review » Needs work
cs_shadow’s picture

Status: Needs work » Needs review
FileSize
1.5 KB

Changed URLs to the ones suggested in #11. This patch is against 7.x-2.x, can make the same for other versions if this works fine.

AdamPS’s picture

I confirm the URLs in #11 are good.
I was unable to apply the patch in #12 to 7.x-2.1 as it seems to start from a base file with different URLs.

Here's an attempt at a new patch - apologies if I did it wrong, it's my first uploaded Drupal patch.

bkonetzny’s picture

The new URL http://browscap.org/version didn't work for me, as it contains a time info and not a version number. Correct one (as suggested in #2232795: browscap cache data URL changed!) would be http://browscap.org/version-number.
I updated the patch for 7.x to fix this.

quotesBro’s picture

#14 worked for me, thanks.

Heorhi Lazarevich’s picture

#14 worked for me with Browscap 7.x-2.1
Thank you, Bastian

ADrupalUser’s picture

Unfortunately #14 is not working for me. I used a clean install of 7.x-2.1 and applied the patch. After activating the module I got a 503 error. I then went to the configuration page and attempted refreshing the browscap data and got the same 503 error.

alexander.sibert’s picture

I downloaded now http://browscap.org/stream?q=BrowsCapINI without PHP and it worked but is that now wrong? The other URL http://browscap.org/stream?q=Full_PHP_BrowsCapINI gave me time out. Hope the module will be fixed.

shafiqhossain’s picture

Browscap have moved their domain and url, the module did not update those url in the import.inc file. To me it was not working until i changed those:

//$current_version = drupal_http_request('http://tempdownloads.browserscap.com/versions/version-number.php');
$current_version = drupal_http_request('http://browscap.org/version-number');

Similarly, for download, url will be:
//$browscap_data = drupal_http_request('http://tempdownloads.browserscap.com/stream.php?BrowsCapINI');
$browscap_data = drupal_http_request('http://browscap.org/stream?q=BrowsCapINI');

Another issue is related to drupal_http_request() function, about timeout, which is a core drupal bug, timeout for fetching is set 30 sec, which is fixed in Drupal 8. So if you see the time out problem, change the url as follows:
$options = array(
'method' => 'GET',
'timeout' => 3000,
);
$browscap_data = drupal_http_request('http://browscap.org/stream?q=BrowsCapINI', $options);

alexander.sibert’s picture

Yeah i got also a time out issue.

jeroen_drenth’s picture

#19 works for me!

AohRveTPV’s picture

These patches update the download URLs and set what I think is a reasonable timeout for the HTTP requests.

A timeout is needed because the filesize is ~6500 KB, which with the default timeout of 30 s requires bandwidth of at least 6500 / 30 = ~220 KB/s. That is a lot of bandwidth to expect from server and client.

I set a conservative timeout of 10 min (600 s), which means a bandwidth requirement of ~10 KB/s. #19/#21 set the timeout to 50 min (3000 s), but that seems high.

A potential problem with increasing the timeout is that for a site using the built-in visit-triggered cron, the visitor's request could block while waiting on the HTTP request to finish. Unfortunately, drupal_http_request() does not support gzip compression, which would allow us to reduce the download size by an order of magnitude.

The prior patches disagreed over which download URL to use. I chose http://browscap.org/stream?q=PHP_BrowsCapINI. I believe we want the PHP URL because the downloaded data is parsed by either PHP parse_ini_string() or parse_ini_file(). Alternatively, we could use http://browscap.org/stream?q=Full_PHP_BrowsCapINI, which has the most browser information, but switching to a different, 2x bigger dataset is perhaps a different issue than getting the same downloads back working.

AohRveTPV’s picture

#22 does not specify the GET method as does #19/#21. It is unnecessary because GET is the default.

AohRveTPV’s picture

This patch uses gzip decompression if available to speed up the download by 10-20x, and avoid increasing the drupal_http_request() timeout. gzip decompression is only available in PHP >= 5.4.

Unfortunately, http://browscap.org does not support Accept-Encoding: deflate. If it did, I believe we could use compressed downloads on every supported PHP version via gzinflate(), and avoid changing the default drupal_http_request() timeout.

We could open a request with the Browscap project to add support for deflate, if it seems like a sensible thing to do. They are using Apache so it should be just a matter of enabling mod_deflate.

AohRveTPV’s picture

Sorry, the patch in #24 had a crucial typo. This updated patch should work, though I cannot currently test because I've been temporarily banned from browscap.org for too many requests.

greggles’s picture

The patch in #25 looks like a good solution to me. Anyone else want to review and provide feedback?

alexander.sibert’s picture

I can test and review it.

AohRveTPV’s picture

With Wireshark I verified that gzip compression is used if available, and that the download is much smaller (~400KB vs. ~6500KB).

Interestingly, the php-browscap project linked from http://browscap.org uses file_get_contents() to perform the download. I wonder if this handles compression transparently, or if they are not using compression.

Even with compression, it seems using visit-triggered cron (the Drupal default) should be avoided with this module. Otherwise, in theory, you will get users that are sporadically hit with 10+ second requests. Populating the {browscap} table takes a number of seconds on my database, so it's not just due to the download. Perhaps the README.txt should be updated to warn people to trigger cron externally if they are using the automatic downloads feature of this module.

AohRveTPV’s picture

Will open a new issue to deal with the last paragraph of #28. This issue should probably be focused just on getting things back to working as before.

Issue links:
#2287153: Always use compression for downloads
#2287159: Disable automatic downloads by default, provide drush integration

jorisx’s picture

#25 seems to works fine until now
tested on:
Drupal 7.28
PHP 5.3.28
Browscap-7.x-2.1

AohRveTPV’s picture

Thanks, jorisx. Is there anyone who could test #25 on PHP >=5.4, which would test the other case where gzip compression is used? I have tested that case myself, but not sure if it is enough to set this to RTBC.

ADrupalUser’s picture

#25 is not working for me with php 5.4.29. I'm still getting a timeout error.

AohRveTPV’s picture

ADrupalUser, does it work if you change:

$options = array(
  'headers' => array('Accept-Encoding' => 'gzip'),
);

to:

$options = array(
  'headers' => array('Accept-Encoding' => 'gzip'),
  'timeout' => 600,
);

What is the error message you are getting?

Edit: Deleted incorrect statement I made. It probably shouldn't take longer than 30 seconds if using gzip, as it only requires ~10KB/s download speed. Importing the data into the database should take the majority of the time. Maybe the web server is timing out somehow?

greggles’s picture

Version: 7.x-2.x-dev » 8.x-1.x-dev
Status: Needs review » Patch (to be ported)

I committed this to 7.x-2.x-dev. Folks should test that out in ~6 hours and report results here.

It's now "to be ported" to 8.x and/or 6.x. I don't actively use either of those branches and 8.x is somewhat out of synch with both core 8.x and the 7.x-2.x branch of this so I could also see marking this as "fixed" and skipping those other branches.

  • greggles committed 692b635 on 7.x-2.x authored by AohRveTPV
    Issue #2171549 by AohRveTPV: Update Browscap.ini file url http://...
AohRveTPV’s picture

Version: 8.x-1.x-dev » 6.x-2.x-dev
Status: Patch (to be ported) » Needs review
FileSize
1.85 KB

Thanks, greggles. Port of #25 to 6.x-2.x.

greggles’s picture

Version: 6.x-2.x-dev » 8.x-1.x-dev

Okeydoke. Committed to 6.x-2.x. Thanks AohRveTPV!

greggles’s picture

Status: Needs review » Patch (to be ported)

  • greggles committed 15bd410 on 6.x-2.x authored by AohRveTPV
    Issue #2171549 by AohRveTPV: Update Browscap.ini file url http://...
AohRveTPV’s picture

Status: Patch (to be ported) » Reviewed & tested by the community

8.x-1.x appears to be an unmodified branch of 7.x-2.x which is a few commits behind. 8.x-1.x could be resynchronized with 7.x-2.x by applying a few commits:
http://cgit.drupalcode.org/browscap/log/?h=7.x-2.x

Or it could be left out of date as greggles suggests. 8.x-1.x will need to be changed significantly to make it actually work under D8, anyway.

Since 8.x-1.x is the same code as 7.x-2.x, the vetted patch for 7.x-2.x in #25 should apply to 8.x-1.x, so setting to RTBC.

greggles’s picture

Version: 8.x-1.x-dev » 7.x-2.x-dev
Status: Reviewed & tested by the community » Closed (fixed)

Thanks for the research, AohRveTPV.

Given that it appears to be a branch without novel work, I think we should not forwardport things to it and instead just consider this issue to be fixed.

Can anyone else confirm if the latest 7.x-2.x dev fixes this issue?

informatica_cnc’s picture

I patched my D7 browscap to 25 and the download file is successful, but fail at line 103 with function $browscap_data = parse_ini_string($browscap_data, TRUE, INI_SCANNER_RAW);
My php version is 5.5.37. Is there any incompatibility with php 5.5?

Anybody’s picture

Could you please release a new stable version? The last stable is no more usable due to this changed link.

greggles’s picture

@Anybody - I've asked for reviews of the dev version to confirm its working for a broad set of folks (since it does different things depending on your php configuration). Can you please review it? Then I'll create a new release ;)

Anybody’s picture

Sorry, my mistake! Thanks a lot! I've reviewed it in my environments and for me it works great, while the current stable version is completely dead ;)

AohRveTPV’s picture

informatica_cnc: There is no PHP 5.5.37. The latest 5.5.x release is 5.5.14. Can you clarify? 5.5.37 might be a MySQL/etc. version number. Also, the actual error message would be helpful. (Or is what you posted the entire error message?)

informatica_cnc’s picture

you're right AohRveTPV, the 5.5 version is about mysql. my PHP version is 5.3.10-1ubuntu3.12 but when I try to "refresh browser data" I have not any error message I get only a blank page.

Thanks

greggles’s picture

I created https://www.drupal.org/node/2300257

Even if its broken it can't be more broken than what we currently have ;)

informatica_cnc’s picture

Finally I updated my php from 5.3 to 5.5 and I fix the problem. Thanks for all AohRveTPV

Anybody’s picture

FYI: We're using PHP 5.3 and it works fine with the latest .dev. Maybe you had safe_mode active in PHP 5.3 or something like this which was indirectly solved by the upgrade.

rfay’s picture

The 6.x-2.x-dev is working for me. php 5.3.10/Ubuntu/php-fpm

izmeez’s picture

The 6.x-2.x-dev release is not working for me. Refresh browscap data returns "Couldn't check version: Gone". php 5.3.28/centos

my bad, 6.x-2.x-dev release, when that's what you really got :-), works. May need php memory limit increased to 192M.

izmeez’s picture

I wonder if it would be helpful or unnecessary to add in the Readme.txt that the php memory_limit needs to be a minimum of 192M when enabling the module to avoid WSOD from memory exhausted.

greggles’s picture

@izmeez - Any new work should be filed as a new issue. Work on reducing the memory issue is in #2418473: Allowed memory size and it's making some good progress, as far as I can tell. The latest 7.x-2.x code no longer does an import during install, so that issue is "fixed".

greggles’s picture

Oh, I realize now that you're talking about the 6.x branch. @aohrvetpv is now the maintainer of that branch, and can update on the status/plans for that. My advice about "new problem, new issue" stands ;)

izmeez’s picture

Sorry, my mistake, especially as it is closed :{
Thanks for linking to the other issue, I can also follow for D7.

izmeez’s picture

Issue #2418473-21: Allowed memory size includes patch for 6.x-2.x memory size. Thanks.