Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-81248

parseMaxTimeMS may narrow values of maxTimeMSOpOnly

    XMLWordPrintableJSON

Details

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Minor - P4 Minor - P4
    • None
    • None
    • None
    • Query Optimization
    • ALL

    Description

      When used to parse a field with the name maxTimeMSOpOnly, parseMaxTimeMS increases the max permitted value beyond INT_MAX.

      However, the returned value is of type int - the result will be narrowed long long -> int.

      StatusWith<int> parseMaxTimeMS(BSONElement maxTimeMSElt) {
      ...
          const long long maxVal = maxTimeMSElt.fieldNameStringData() == kMaxTimeMSOpOnlyField
              ? (long long)(INT_MAX) + kMaxTimeMSOpOnlyMaxPadding
              : INT_MAX;
      ...
          if (maxTimeMSLongLong < 0 || maxTimeMSLongLong > maxVal)
              return Status(ErrorCodes::BadValue, ...);
      ..
          return StatusWith<int>(static_cast<int>(maxTimeMSLongLong));
      }
      

      The justification for this leeway:

      src/mongo/db/query/max_time_ms_parser.h

      43
      // A constant by which 'maxTimeMSOpOnly' values are allowed to exceed the max allowed value for
      44
      // 'maxTimeMS'.  This is because mongod and mongos server processes add a small amount to the
      45
      // 'maxTimeMS' value they are given before passing it on as 'maxTimeMSOpOnly', to allow for
      46
      // clock precision.
      47
      static constexpr auto kMaxTimeMSOpOnlyMaxPadding = 100LL;
      

      Given this, if maxTimeMS is provided INT_MAX, any maxTimeMSOpOnly derived from increasing this value slightly will be parsed as a very negative int value.

      The only use of this method on a field named maxTimeMSOpOnly identified so far is:

      src/mongo/db/service_entry_point_common.cpp

          void ExecCommandDatabase::_initiateCommand() {
      ...
              const auto maxTimeMS =
                  Milliseconds{uassertStatusOK(parseMaxTimeMS(cmdOptionMaxTimeMSField))};
              const auto maxTimeMSOpOnly =
                  Milliseconds{uassertStatusOK(parseMaxTimeMS(maxTimeMSOpOnlyField))};
       
              if ((maxTimeMS > Milliseconds::zero() || maxTimeMSOpOnly > Milliseconds::zero()) &&
                  command->getLogicalOp() != LogicalOp::opGetMore) {
                 ...
       
                  if (!ignoreMaxTimeMSOpOnly && maxTimeMSOpOnly > Milliseconds::zero() &&
                      (maxTimeMS == Milliseconds::zero() || maxTimeMSOpOnly < maxTimeMS)) {
                      opCtx->storeMaxTimeMS(maxTimeMS);
                      opCtx->setDeadlineByDate(startedCommandExecAt + maxTimeMSOpOnly,
                                               ErrorCodes::MaxTimeMSExpired);
                  } else if (maxTimeMS > Milliseconds::zero()) {
                      opCtx->setDeadlineByDate(startedCommandExecAt + maxTimeMS,
                                               ErrorCodes::MaxTimeMSExpired);
                  }
              }
      

      This compares > 0, so may treat this situation as if maxTimeMSOpOnly were 0 or unset.

      Given INT_MAX milliseconds is around 25 days, it's possible this has no real-world impact.

      Attachments

        Activity

          People

            backlog-query-optimization Backlog - Query Optimization
            james.harrison@mongodb.com James Harrison
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated: