Apache 1.2 is so long gone that even its memory is forgotten. There are no webservers accepted. We do not use any Apache 1.3 specific features. There are ISPs that do very strange things to Apache version strings.

So, we have code that buys us nothing but causes problems. Begone!

CommentFileSizeAuthor
#7 remove_webserver_checking_d6.patch2.33 KBpancho
#3 remove_webserver_checking_01.89 KBchx
remove_webserver_checking1.88 KBchx

Comments

dries’s picture

dries commented

The check is mostly redundant but not 100%. However, displaying the Apache version string on the status page is still useful.

Steven’s picture

Steven commented
Title: Web server version checker is not needed » Web server version checker needs love
Status: Needs review » Needs work

The item on the status report should definitely stay, as this is useful debugging information to copy/paste around. Whether the version check is needed is something else.

I agree that in its current form, the version check is a bit redundant. However, the note about unsupported web servers (e.g. lighttpd, iis) is useful.

Instead, we should augment it to recognize other platforms too, and do a useful version check there.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new remove_webserver_checking_01.89 KB

In Drupal 6 we can love this code as much as we want. For now, it's a support burden.

dries’s picture

dries commented

I'm OK with that last patch. Steven?

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: 5.x-dev » 6.x-dev
Status: Needs review » Reviewed & tested by the community

Looks okay to me and I have this issue on localhost (apparently I am simply running 'Apache' with no version).

chx, Dries, and myself agreeing should be enough without Steven. It can always be patched later if needed.

Committed to 5.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

No longer applies against CVS HEAD.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Title: Web server version checker needs love » Regression: Remove web server version checker
Status: Needs work » Patch (to be ported)
StatusFileSize
new remove_webserver_checking_d6.patch2.33 KB

This patch has been committed to the 5.x branch, but not to HEAD, therefore we are having a regression in D6.

Attached patch ports this to HEAD. #105853 moved the requirements constants to system.module, so I fixed it there as well.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Patch (to be ported) » Needs review

Well, "to be ported" is when the patch is not there yet for the target version. When there is a patch, we use "patch needs review".

ax’s picture

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

works as advertised on CVS HEAD as of today. please commit - this "Unable to determine your web server type and version. Drupal might not work properly." even though you are using the most recent Apache (with ServerTokens set to "Prod") really isn't helpful.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Agreed, committed, thanks.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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