If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

checksetup should detect when run via a web server (cgi) and fail with a sane error

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Installation & Upgrading
P1
enhancement
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: glob, Assigned: Frédéric Buclin)

Tracking

unspecified
Bugzilla 4.2
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.34 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
checksetup should check if it's being run from a web browser and bail out early with a sane error (with an appropriate content-type header).

Updated

8 years ago
Hardware: x86 → All
(Assignee)

Comment 1

7 years ago
Yeah, someone complained again today in the support mailing-list that http://.../checksetup.pl wasn't working fine. As they seem unable to read the documentation before installating Bugzilla, the error message will have to tell them where the doc is and that it must be run from a shell. IMO, something we should have for 4.0.
Priority: -- → P1
Target Milestone: --- → Bugzilla 4.0
(Assignee)

Comment 2

7 years ago
Created attachment 468805 [details] [diff] [review]
patch, v1
Assignee: installation → LpSolit
Status: NEW → ASSIGNED
Attachment #468805 - Flags: review?(mkanat)

Comment 3

7 years ago
Comment on attachment 468805 [details] [diff] [review]
patch, v1

>Index: checksetup.pl
> [snip]

  All of this should be in Bugzilla::Install::Util, and should just be called as a function in checksetup.pl.

>+    # Maybe CGI.pm is not installed.

  Actually, CGI.pm is always installed--it comes with Perl 5.8. So we should just use CGI::Carp here.

> Also, as the web server may not
>+    # have access to templates yet, we hardcode the error message here.

  In the interest of localization, I think that we should only use a hardcoded message as a fallback.

>+    <title>Installation Failed</title>

  "Installation Failed" probably isn't the right title. Perhaps something more like "You Cannot Run This Script In a Web Server".

>+      You <b>must not</b> execute this script from your web browser.
>+      To install or upgrade Bugzilla, run this script from
>+      the command-line (e.g. <tt>bash</tt> on Linux

  I think we should say "bash or ssh", because a lot of the people who are going to try to do this have shared hosting and won't know what bash is.

>+      For more information on how to install Bugzilla, please
>+      <a href="http://www.bugzilla.org/docs/tip/en/html/installing-bugzilla.html">
>+      read the documentation</a> available on the official Bugzilla website.

  That information could change drastically between versions, so we probably shouldn't link to the tip docs. Instead, we should just link to the docs page and tell them where to go.
Attachment #468805 - Flags: review?(mkanat) → review-
(Assignee)

Comment 4

7 years ago
(In reply to comment #3)
>   Actually, CGI.pm is always installed--it comes with Perl 5.8. So we should
> just use CGI::Carp here.

Well, it doesn't come with Perl 5.10.1 on Mandriva Linux. It's a separate package which I had to install separately. So we shouldn't assume it's always available. I can add an eval {require CGI::Carp} if we want the code complexity.


>   In the interest of localization, I think that we should only use a hardcoded
> message as a fallback.

Does it really worth the code complexity for something which doesn't happen frequently?

Comment 5

7 years ago
(In reply to comment #4)
> Well, it doesn't come with Perl 5.10.1 on Mandriva Linux. It's a separate
> package which I had to install separately.

  Well, that's an error on the packager's part. CGI::Carp is a part of Perl since 5.8.

  http://perldoc.perl.org/5.8.8/CGI/Carp.html

  If there's no CGI::Carp installed, then Perl is not correctly installed, and we don't have to worry about that. (We certainly don't worry about it elsewhere in the code, with things like Scalar::Util.)

  As far as having the hardcoded fallback to install_string...if the webserver can't read the templates, it probably can't read the modules either, and so checksetup.pl won't even get to this point. So we should probably just use install_string.
(Assignee)

Comment 6

7 years ago
While working on this bug, I noticed two things:
- First, I don't see why we should use CGI::Carp. We don't want to throw a software error, but simply display a text in the web browser. Moreover, fatalsToBrowser is not supported by mod_perl.
- Secondly, I cannot call install_string() so early in checksetup.pl, because we didn't |require Bugzilla;| yet, and so Bugzilla->cgi called within _wanted_languages() is not available. Do we want to eval {Bugzilla-cgi} there, and fallback to CGI->new() if needed?
(Assignee)

Comment 7

7 years ago
Created attachment 482068 [details] [diff] [review]
patch, v2
Attachment #468805 - Attachment is obsolete: true
Attachment #482068 - Flags: review?(mkanat)

Comment 8

7 years ago
Comment on attachment 482068 [details] [diff] [review]
patch, v2

Oh, I like this! this is very nice. :-)
Attachment #482068 - Flags: review?(mkanat) → review+

Updated

7 years ago
Flags: approval+
Target Milestone: Bugzilla 5.0 → Bugzilla 4.2
(Assignee)

Comment 9

7 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified checksetup.pl
modified Bugzilla/Install/Util.pm
modified template/en/default/setup/strings.txt.pl
Committed revision 7527.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.