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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: morrison dot levi at gmail dot com
New email:
PHP Version: OS:

 

 [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 21 17:01:58 2024 UTC