Bug 187843 - Review Request: phpMyAdmin - Web based MySQL browser written in php
Review Request: phpMyAdmin - Web based MySQL browser written in php
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-03 16:57 EDT by Mike McGrath
Modified: 2011-05-30 22:46 EDT (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-10 11:25:32 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
petersen: fedora‑cvs+


Attachments (Terms of Use)
Patch including my personal changes from comment #19 (1.70 KB, patch)
2006-06-21 11:15 EDT, Robert Scheck
no flags Details | Diff

  None (edit)
Description Mike McGrath 2006-04-03 16:57:35 EDT
Spec: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
SRPM: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.0.2-1.src.rpm
Description: 
phpmyadmin is a web based MySQL management tool.  It allows users to use and
administer a MySQL instance.
Comment 1 Greg Swallow 2006-04-03 20:38:55 EDT
Just wanted to make you aware of someone else packaging phpMyAdmin:
http://remi.collet.free.fr/rpms/SRPMS/phpMyAdmin-2.8.0.2-1.fc4.remi.src.rpm
Probably you knew about this one already, but just in case you didn't know, now 
you can collaborate.
Comment 2 Don Russell 2006-04-03 22:08:41 EDT
Added myself to the "interested parties" list. :-)
Comment 3 Michael Fleming 2006-04-04 04:43:55 EDT
See also
http://www.enlartenment.com/packages/fedora/5/x86_64/repodata/repoview/phpmyadmin-0-2.8.0.2-1.fc5.mf.html
for my contribution.

Not a review per-se, but some comments after an elementary look at the package:

- Feel free to be more verbose in the %description and summary - you can do far
more than simply browse databases with it, it's a full admin app after all.
- The source package doesn't ship a default config.inc.php file, users would
normally create one on the fly. Yours is a good base but I would personally
create a %{_localstatedir}/www/phpmyadmin/scripts/config dir per the install
instructions (unfortunately they want it world writable *sigh*) allowing the
user to create the config via the included /scripts/setup.php interface without
much hassle.
- I would not restart apache in %post/%postun - either issue a reload or leave
it for the administrator to perform manually. Look at the services related
scriptlet at
http://www.fedoraproject.org/wiki/ScriptletSnippets#head-a1898cc5440737a43df9b41b9ad6d7e7c7c3e6e2

Michael.
Comment 4 Dmitry Butskoy 2006-04-04 06:35:41 EDT
Don't place files under /var/www . Actually! It is not a VARiable stuff. Place
under /usr/share/phpmyadmin and use Apache's alias directive...

See "squirrelmail" package from the Core, or "phpldapadmin" from the Extras for
an example.
Comment 5 Mike McGrath 2006-04-04 10:28:43 EDT
Spec: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
SRPM: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.0.2-2.src.rpm

- Moved everything to %{_datadir}
- Moved config file to /etc/
- Used description from phpMyAdmin project info
Comment 6 Dmitry Butskoy 2006-04-04 11:33:43 EDT
IMO it is better to auto-generate phpMyAdmin.htaccess file somewhere in the
spec, i.e.:


cat >phpMyAdmin.htaccess <<EOF
....
....
EOF

You can avoid hard-coded /usr/share in this case (using %{_datadir} instead),
and such a way seems to be more clear...
Comment 7 Mike McGrath 2006-04-04 15:48:21 EDT
I've done that in the past but a couple of reviewers convinced me to leave it as
a source file.  Here's some updates - stupid things I missed earlier.

SPEC: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
SRPM: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.0.2-3.src.rpm

- Made config files actually configs
- Moved doc files to the doc dir
Comment 9 Mike McGrath 2006-04-06 14:00:53 EDT
Updated package to fix issues brought up in Comment #8, thanks Ville Skyttä!

SPEC: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
SRPM: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.0.3-1.src.rpm
Comment 10 Remi Collet 2006-05-07 05:27:04 EDT
The only Requires in the spec is httpd.

To run phpMyAdmin also need php, php-mysql and php-mbstring.
Comment 11 Mike McGrath 2006-05-07 13:52:28 EDT
Added php-mysql and php-mbstring to Requires:

SPEC: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
SRPM: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.0.3-2.src.rpm
Comment 12 Michael A. Peters 2006-05-07 15:11:10 EDT
Not a formal review - but it looks good to me.

I don't know if this is worth worrying about, but it is possible to use
phpMyAdmin without httpd (the fedora apache package).

It can also be used with thttpd, for example, if thttpd is compiled with the
proper php support.

At a previous company I worked for, we would do that on MySQL servers that were
not web servers, and didn't need the weight of apache installed just to use a
web based admin tool for MySQL.

I don't think the thttpd in Extras is compiled with php support though (is it?)
Comment 13 Roy-Magne Mo 2006-05-12 14:21:48 EDT
Seems like phpmyadmin has been updated:
http://www.phpmyadmin.net/home_page/downloads.php?relnotes=0

Like Michael A. Peters suggested, maybe require webserver instead of httpd. And
even though a couple of php-modules is a requirement, php should be a
requirement too. 

I've tested the package on a FC5-host with mysql 5.0.18, seems to work very well.
Comment 14 Don Russell 2006-05-13 09:48:59 EDT
Re comment #13:

I woud expect the individual php-modules to have their own set of dependencies -
primarily php itself.

Is it necessary for phpmyadmin to also specify that dependency?

I think dependencies should be as specfic as possible, not all-encompassing.
There are processes then which resolve, iteratively or recursively, all
dependencies.
Comment 15 Roy-Magne Mo 2006-05-13 10:53:51 EDT
(In reply to comment #14)
> Re comment #13:
> 
> I woud expect the individual php-modules to have their own set of dependencies -
> primarily php itself.
> 
> Is it necessary for phpmyadmin to also specify that dependency?

Seems like cacti, phpldapadmin, mediawiki has a both requires on php-modules and
php itself. This way you can specify a version of php to require. 

Comment 16 Don Russell 2006-05-13 12:00:30 EDT
(In reply to comment #15)and
> This way you can specify a version of php to require. 

By all means, if a specific version of php is required, then list it as a
requirement. Is that the case? 

I'm just saying don't list php as a requirement just because php-modules requies
php. List the fewest requirements/dependencies as possible, and still be
complete - let the dependency-resolver process do its job. :-)

A general rule of thumb: do not list a dependency that is already a dependency
of one of your dependents.

So, if A depends on B, and B depends on C, it is not necessary (nor desirable)
to list C as a dependent of A.

If some special version of C (Cs) is required by A, then yes:

A requires B
A requires Cs
B requires C

Anyway... it's not my intention to take up a lot of bandwidth on 'depency
theory'. :-)

Comment 17 Roy-Magne Mo 2006-05-13 12:32:08 EDT
(In reply to comment #16)
> Anyway... it's not my intention to take up a lot of bandwidth on 'depency
> theory'. :-)

yes, I subscribe to your theory, and think you are right :) 
Comment 18 Mike McGrath 2006-05-13 16:23:42 EDT
Figure I'd chime in on this.  The packaging guidelines are fairly vague on this,
the closest thing I can find only relates to libraries:

"RPM has very good capabilities of automatically finding dependencies for
libraries and eg. Perl modules. In short, don't reinvent the wheel, but just let
rpm do its job. There is usually no need to explicitly list eg. Requires:
XFree86 when the dependency has already been picked up by rpm in the form of
depending on libraries in the XFree86 package."

In the interest of consistancy I'll add php to the requires section.  (I'm also
the packager of Cacti and it is in there)

SPEC: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
SRPM: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.0.4-1.src.rpm

Also, all my test show that php will actually pull httpd in every time so I'm
not sure using webserver makes any difference right now but it might in the future.

changelog:
- New upstream version
- added php as requires
- changed httpd to webserver as a requires
Comment 19 Robert Scheck 2006-06-21 11:13:08 EDT
There are minimum requirements for phpMyAdmin as per README, changing to:
Requires: webserver, php >= 4.1.0, php-mysql >= 4.1.0, php-mbstring >= 4.1.0, 
mysql-server >= 3.23.32

I also would suggest adding the following to Source2, because of rpmlint error:
E: phpMyAdmin htaccess-file /usr/share/phpMyAdmin/libraries/.htaccess

--- snipp ---
# This directory does not require access over HTTP - taken from the original
# phpMyAdmin upstream tarball
#
<Directory /usr/share/phpMyAdmin/libraries>
    Order Deny, Allow
    Deny from All
    Allow from None
</Directory>

# This configuration prevents mod_security at phpMyAdmin directories from
# filtering SQL etc.
#
<IfModule mod_security.c>
    <LocationMatch "/phpMyAdmin/(.+)">
        SecFilterInheritance Off
    </LocationMatch>
</IfModule>
--- snapp ---

Personally I'm not happy with having phpMyAdmin restricted for localhost only. 
Of course, on the other hand I can see the security aspect, too.

Another thing is, for my personal use, I didn't allow logins at phpMyAdmin
using the "mysql" user which has normally no password. At least I changed 
authentication method to "http" to allow every mysql user a login. Just have
a look to the patch to see what I did...

Oh and there are two additional rpmlint messages:
W: phpMyAdmin incoherent-version-in-changelog 2.8.0.3-1 2.8.0.4-1
E: phpMyAdmin zero-length /usr/share/doc/phpMyAdmin-2.8.0.4/Documentation.txt

Hm, when symlinking the configuration file, you also should add a "Options 
+FollowSymLinks" or similar to Source2, otherwise there can be a Forbidden with 
having a strict Apache configuration, IIRC.

And finally, isn't phpMyAdmin 2.8.1 latest version (as per www.phpmyadmin.net)?
Comment 20 Robert Scheck 2006-06-21 11:15:03 EDT
Created attachment 131286 [details]
Patch including my personal changes from comment #19
Comment 21 Mike McGrath 2006-07-04 01:03:46 EDT
Ok, I think most of these are added.

Changelog
- New upstream version
- Added httpd/conf.d/phpMyAdmin.conf restrictions
- Upstream now uses http authentication by default
- Added versions to requires

We don't need FollowSymLinks because php's actually following the links, not apache.

I'm still pondering whether or not to restrict the mysql user.  I have a feeling
I'll get bug reports about it.  I don't necessarily think its a bad idea.  

SPEC: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
SRPM: http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.2-1.src.rpm

62905d19638d6b62140e0544028df886  phpMyAdmin.spec
1629d2755b688f0a3f64b0bf6f3b2bb3  phpMyAdmin-2.8.2-1.src.rpm
Comment 22 Kayvan Sylvan 2006-07-06 02:55:02 EDT
Question: Why did you use the "mysql" python extension rather than mysqli?

When I tried your RPM on FC4, I got the following result:

[root@satyr conf.d]# service httpd start
Starting httpd:                                            [FAILED]
[root@satyr conf.d]# mv phpMyAdmin.conf /tmp
[root@satyr conf.d]# service httpd start
Starting httpd:                                            [  OK  ]

So, there's something in the phpMyAdmin.conf file that apache did not like. Will
try to track it down...
Comment 23 Kayvan Sylvan 2006-07-06 08:06:01 EDT
Looks like the stock httpd-2.0.54-10.3 did not like the space in the "Order
Deny, Allow" line. Making that be "Order Deny,Allow" fixed my httpd startup problem.
Comment 24 Mike McGrath 2006-07-06 10:20:30 EDT
Doah!  Forgot to update that source file.

http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin.spec
http://mmcgrath.net/~mmcgrath/phpMyAdmin/phpMyAdmin-2.8.2-2.src.rpm

As for why I used php-mysql.  Just habit I guess, I've always used it for my php
mysql support.
Comment 25 Stewart Adam 2006-09-17 22:16:12 EDT
rpmlint on SRPM:
W: phpMyAdmin macro-in-%changelog _datadir
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.
Comment 26 Stewart Adam 2006-09-17 22:21:08 EDT
Just to clairfy, I'm not a reviewer but I like the idea of phpMyAdmin in Extras
and I'm trying to help out...
I rebuilt your SRPM and this is rpmlint on the binary RPM:
W: phpMyAdmin symlink-should-be-relative /usr/share/phpMyAdmin/config.inc.php
/etc/phpMyAdmin/config.inc.php
Absolute symlinks are problematic eg. when working with chroot environments.
Comment 27 Ville Skyttä 2006-10-03 12:06:31 EDT
CVE-2006-5117: PHPMyAdmin Multiple Unspecificied Vulnerabilities
http://nvd.nist.gov/nvd.cfm?cvename=CVE-2006-5117
Comment 28 Ville Skyttä 2006-10-03 12:08:29 EDT
Comment 27's "description" was actually for CVE-2006-5116:
http://nvd.nist.gov/nvd.cfm?cvename=CVE-2006-5116
Comment 29 Rex Dieter 2006-10-03 12:51:48 EDT
Mike, update to the latest available version, and I could review this.
Comment 31 Ville Skyttä 2006-11-04 04:30:30 EST
Cross site scripting vulnerability in 2.6.4-2.9.0.2:
http://nvd.nist.gov/nvd.cfm?cvename=CVE-2006-5718
Comment 32 Rex Dieter 2006-11-07 09:20:48 EST
OK, I was bad, and got busy... I'll try to review this before the FPB meeting 
this morniing.
Comment 34 Rex Dieter 2006-11-07 09:33:06 EST
2.9.1 is a pre-release, so the Release tag should reflect that, use something like:

Release: 0.1.rc2
Comment 35 Rex Dieter 2006-11-07 09:53:56 EST
OK, rest of the package is sane, pretty straight-forward and simple, rpmlint-clean.

MUSTFIX: Release tag must be of the form 
0.1.<alpha|beta|rc>

I trust you'll fix that before importing, else I'll mark this APPROVED.
Comment 36 Mike McGrath 2006-11-10 11:25:32 EST
imported.
Comment 37 Robert Scheck 2011-05-29 17:52:14 EDT
As far as I can see, our pkgdb isn't able to update the short description, so
let's do it here:


Package Change Request
======================
Package Name: phpMyAdmin
Short Description: Handle the administration of MySQL over the World Wide Web
Comment 38 Jens Petersen 2011-05-30 22:46:54 EDT
Fixed in pkgdb.

Note You need to log in before you can comment on or make changes to this bug.