Bug 187843

Summary: Review Request: phpMyAdmin - Web based MySQL browser written in php
Product: [Fedora] Fedora Reporter: Mike McGrath <imlinux>
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: cweyl, dmitry, fedora, fedora, kayvansylvan, mfleming+rpm, mpeters, rdieter, redhat-bugzilla, rmo, s.adam
Target Milestone: ---Flags: petersen: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-11-10 16:25:32 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    
Attachments:
Description Flags
Patch including my personal changes from comment #19 none

Description Mike McGrath 2006-04-03 20:57:35 UTC
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-04 00:38:55 UTC
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-04 02:08:41 UTC
Added myself to the "interested parties" list. :-)

Comment 3 Michael Fleming 2006-04-04 08:43:55 UTC
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 10:35:41 UTC
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 14:28:43 UTC
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 15:33:43 UTC
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 19:48:21 UTC
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 18:00:53 UTC
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 09:27:04 UTC
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 17:52:28 UTC
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 19:11:10 UTC
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 18:21:48 UTC
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 13:48:59 UTC
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 14:53:51 UTC
(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 16:00:30 UTC
(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 16:32:08 UTC
(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 20:23:42 UTC
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 15:13:08 UTC
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 15:15:03 UTC
Created attachment 131286 [details]
Patch including my personal changes from comment #19

Comment 21 Mike McGrath 2006-07-04 05:03:46 UTC
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 06:55:02 UTC
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 12:06:01 UTC
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 14:20:30 UTC
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-18 02:16:12 UTC
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-18 02:21:08 UTC
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 16:06:31 UTC
CVE-2006-5117: PHPMyAdmin Multiple Unspecificied Vulnerabilities
http://nvd.nist.gov/nvd.cfm?cvename=CVE-2006-5117

Comment 28 Ville Skyttä 2006-10-03 16:08:29 UTC
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 16:51:48 UTC
Mike, update to the latest available version, and I could review this.

Comment 31 Ville Skyttä 2006-11-04 09:30:30 UTC
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 14:20:48 UTC
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 14:33:06 UTC
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 14:53:56 UTC
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 16:25:32 UTC
imported.

Comment 37 Robert Scheck 2011-05-29 21:52:14 UTC
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-31 02:46:54 UTC
Fixed in pkgdb.