php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #60341 SplFixedArray should throw specific exceptions.
Submitted: 2011-11-19 21:31 UTC Modified: 2020-10-01 01:40 UTC
Votes:5
Avg. Score:4.0 ± 0.9
Reproduced:4 of 4 (100.0%)
Same Version:4 (100.0%)
Same OS:3 (75.0%)
From: morrison dot levi at gmail dot com Assigned: levim (profile)
Status: Closed Package: SPL related
PHP Version: 5.3 OS: irrelevant
Private report: No CVE-ID: None
 [2011-11-19 21:31 UTC] morrison dot levi at gmail dot com
Description:
------------
SplFixedArray thankfully throws exceptions when you try to do incorrect things 
with indices.  However, the types of exceptions are just too generic.  If I give 
the wrong type of index, that's a logic error (I'd expect InvalidArgument or at 
least something that inherits from LogicError to be thrown).  If I give an index 
that's that's a valid type but doesn't exist, I'd expect an OutOfBoundsException 
to be thrown.  Instead I get a generic RuntimeException.

I should expect because they are very different problems that I would at least 
get a distinguishing message between the two.  However, I get the same 
descriptions:  'Index invalid or out of range'.  The very message suggests they 
should be different exceptions.

The first fix would sort-of break backwards compatibility: throw an 
InvalidArgumentException for things of the wrong type.

The second fix, throw OutOfBoundsException on incorrect index, could be 
implemented and keep backwards compatibility.

Test script:
---------------
$fa = new SplFixedArray(1);

$fa[new StdClass()]; //expect InvalidArgumentException or perhaps OutOfRangeException

$fa[2] = 'james'; // expect OutOfBoundsException

Expected result:
----------------
I expect $fa[new StdClass] to throw an InvalidArgumentException, not a 
RuntimeException

I expect $fa['2'] to throw OutOfBoundsException not a RuntimeException.

Actual result:
--------------
Both throw RuntimeExceptions.

Patches

SplFixedArrayOptimizations (last revision 2011-12-12 23:21 UTC by morrison dot levi at gmail dot com)
SplFixedArrayNoLongerAcceptsStrings (last revision 2011-12-12 21:08 UTC by morrison dot levi at gmail dot com)
SplFixedArrayThrowsSpecificExceptions (last revision 2011-11-22 21:54 UTC by morrison dot levi at gmail dot cm)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-11-21 17:36 UTC] morrison dot levi at gmail dot com
This should really be titled 'SplFixedArray should throw specific exceptions'
 [2011-11-21 17:36 UTC] morrison dot levi at gmail dot com
-Summary: SplFixedArray throws generic exceptions. +Summary: SplFixedArray should throw specific exceptions. -PHP Version: Irrelevant +PHP Version: 5.3
 [2011-11-21 20:15 UTC] felipe@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: colder
 [2011-11-22 19:46 UTC] morrison dot levi at gmail dot com
I have a patch nearly ready for this.  It's at home, not on this machine, but I 
noticed that it got assigned, so I thought it was worth mentioning.

Also, I think that:

$fixedArray[] = ''; 

Should throw OverflowException instead of RuntimeException. See 
http://stackoverflow.com/questions/8219158/correct-exception-type-for-adding-to-
an-array-when-it-isnt-allowed and 
http://php.net/manual/en/class.overflowexception.php
 [2011-11-22 21:56 UTC] morrison dot levi at gmail dot com
Note that the proposed patch isn't perfect.  Using a string index that is not 
numeric will throw OutOfBoundsException instead of InvalidArgumentException.  I 
haven't figured out how to do that in C yet, hopefully I'll figure it out soon or 
someone else knows how and can submit the patch.
 [2011-12-12 21:16 UTC] morrison dot levi at gmail dot com
The proposed patch addresses all the issues I have submitted.  However, it is 
important to note that it breaks backwards compatibility in these areas:

 - Passing an Object as a key will throw InvalidArgumentException which does not 
inherit from RuntimeException.
 - Passing a key that does not represent an int will throw 
InvalidArgumentException instead of RuntimeException.

All other changes are backwards compatible (I believe.  Still new to patching 
PHP).
 [2017-10-24 07:25 UTC] kalle@php.net
-Status: Assigned +Status: Open -Assigned To: colder +Assigned To:
 [2018-09-05 16:51 UTC] cmb@php.net
-Assigned To: +Assigned To: levim
 [2020-10-01 01:40 UTC] levim@php.net
-Status: Assigned +Status: Closed
 [2020-10-01 01:40 UTC] levim@php.net
This would be largely backwards incompatible; not worth it.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 07 10:01:28 2024 UTC