PHP Driver
  1. PHP Driver
  2. PHP-554

MongoId should not get constructed when passing in an invalid ID.

    Details

    • Operating System:
      ALL
    • # Replies:
      5
    • Last comment by Customer:
      false

      Description

      You should not be able to construct a new instance of \MongoId with an invalid identifier. Currently, you are able to and it leads to inconsistencies as well as too many invalid queries.

      new \MongoId('i am not a valid id!') -> '509403bd3689a3f942000000' (a valid mongo id)

      Currently, before running all queries, we validate MongoId's, but I feel that this is a hack:

      if ($id !== (new \MongoId($id)->__toString()) throw new InvalidMongoIdException();

      Now we can track when users are passing in invalid ids and stop from unnecessary DB look-ups.

      I propose that a null argument is what creates a new MongoId, and when given an argument, it wraps the given ID in a MongoId object, and when invalid, an exception is thrown.

      Cheers!

        Issue Links

          Activity

          Hide
          Jordan Stout
          added a comment -

          The docs would need to be updated also.

          "If an invalid string is passed to this constructor, a MongoInvalidMongoIdException exception (or whatever) will be thrown."

          And I suppose this isn't a "Major" bug... I guess it depends on use-case... For example, an upsert would create an entirely new document with an incorrect mongo id. Also, if there were references to the correct id, all references may be lost.. For this I'd consider it "Major".

          Show
          Jordan Stout
          added a comment - The docs would need to be updated also. "If an invalid string is passed to this constructor, a MongoInvalidMongoIdException exception (or whatever) will be thrown." And I suppose this isn't a "Major" bug... I guess it depends on use-case... For example, an upsert would create an entirely new document with an incorrect mongo id. Also, if there were references to the correct id, all references may be lost.. For this I'd consider it "Major".
          Hide
          Hannes Magnusson
          added a comment -

          The problem isn't only the fact we silently discard of the string you passed in, but if it happens to be 24 characters we will gladly make it the ID.
          Upon insert however, mongod will get upset and convert it to valid hex - but we have no way of knowing the ID was actually "changed".

          The fix is to raise an error when invalid hex is passed to the constructor, and error out on any other invalid state.

          Show
          Hannes Magnusson
          added a comment - The problem isn't only the fact we silently discard of the string you passed in, but if it happens to be 24 characters we will gladly make it the ID. Upon insert however, mongod will get upset and convert it to valid hex - but we have no way of knowing the ID was actually "changed". The fix is to raise an error when invalid hex is passed to the constructor, and error out on any other invalid state.
          Show
          Hannes Magnusson
          added a comment - https://github.com/mongodb/mongo-php-driver/pull/321
          Hide
          auto
          added a comment -

          Author:

          {u'date': u'2013-03-08T23:38:41Z', u'name': u'Hannes Magnusson', u'email': u'bjori@10gen.com'}

          Message: Fixed PHP-554: MongoId should not get constructed when passing in an invalid ID.
          Branch: master
          https://github.com/mongodb/mongo-php-driver/commit/14f4d5eabc5a0d491ff07b4af80489b96a517759

          Show
          auto
          added a comment - Author: {u'date': u'2013-03-08T23:38:41Z', u'name': u'Hannes Magnusson', u'email': u'bjori@10gen.com'} Message: Fixed PHP-554 : MongoId should not get constructed when passing in an invalid ID. Branch: master https://github.com/mongodb/mongo-php-driver/commit/14f4d5eabc5a0d491ff07b4af80489b96a517759
          Hide
          Jeremy Mikola
          added a comment -

          Hannes Magnusson: I believe this needs a documentation update as well for code 19: http://www.php.net/manual/en/class.mongoexception.php

          Show
          Jeremy Mikola
          added a comment - Hannes Magnusson : I believe this needs a documentation update as well for code 19: http://www.php.net/manual/en/class.mongoexception.php

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since reply:
                1 year, 4 weeks, 4 days ago
                Date of 1st Reply: