Issue 106742: net::URLRequestFtpJob::StartTransaction() - crash

There's a NULL ftp_transaction_factory ptr; a debug build hits two DCHECKs before it hits a NULL ptr. Only affects latest trunk, so change must have been introduced recently.

src\net\url_request\url_request_ftp_job.cc @ line 34
URLRequestJob* URLRequestFtpJob::Factory(URLRequest* request,
                                         const std::string& scheme) {
  DCHECK_EQ(scheme, "ftp");

  int port = request->url().IntPort();
  if (request->url().has_port() &&
    !IsPortAllowedByFtp(port) && !IsPortAllowedByOverride(port))
    return new URLRequestErrorJob(request, ERR_UNSAFE_PORT);

  DCHECK(request->context());
  DCHECK(request->context()->ftp_transaction_factory()); ******** DCHECK ********
  return new URLRequestFtpJob(request);
}

src\net\url_request\url_request_ftp_job.cc @ line 66
void URLRequestFtpJob::StartTransaction() {
  // Create a transaction.
  DCHECK(!transaction_.get());
  DCHECK(request_->context());
  DCHECK(request_->context()->ftp_transaction_factory()); ******** DCHECK ********

  transaction_.reset(
      request_->context()->ftp_transaction_factory()->CreateTransaction()); ******** NULL PTR ********

  // No matter what, we want to report our status as IO pending since we will
  // be notifying our consumer asynchronously via OnStartCompleted.
  SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
  int rv;
  if (transaction_.get()) {
    rv = transaction_->Start(
        &request_info_, &start_callback_, request_->net_log());
    if (rv == ERR_IO_PENDING)
      return;
  } else {
    rv = ERR_FAILED;
  }
  // The transaction started synchronously, but we need to notify the
  // URLRequest delegate via the message loop.
  MessageLoop::current()->PostTask(
      FROM_HERE,
      method_factory_.NewRunnableMethod(
          &URLRequestFtpJob::OnStartCompleted, rv));
}

@skylined: we've typically rated a simple, reliable crash of the browser process as SecSeverity-Low.
Also, don't forget the SecImpacts labels :) In this case, a trunk-only regression qualifies as SecImpacts-None

And Mstone needs to be set to 17 or 18, depending on whether the regression was introduced before or after the M17 branch point. Can you take care of it?

Will do - I didn't get around to doing that yesterday, so I assigned it to me for follow-up today.

Affected: 17+ (dev, canary trunk)
Not affected: 15 (stable) & 16 (beta)

Bulk edit for pending m17 beta release.

Moving bugs marked as Available but not blockers from M17 to M18.  Please move back if you think this is a blocker, and add the ReleaseBlock-Stable label.  If you're able.

skylined have you made any progress on this one?

No, I have no clue what's causing this. ClusterFuzzz can't reproduce.

This chaneg may have something to do with it, joi@ can you have a look?
http://codereview.chromium.org/8769013

According to the first comment on the bug, this was crashing as early as 113143 which is before my change that you linked to (it was committed as r113377).  Was there a special reason you thought r113377 was related?

Nope, I made some assumptions base on svn blame for chrome_url_request_context.cc and the stack at the time of the crash. I'm not familiar with the code - would you know how might help me find out what the problem is here?

This seems like it would be in the net/ code so would look for possibly-related changes around the time the crash got introduced, and perhaps ask one of the net/ folks like wtc@ or eroman@ to take a quick peek.

Thanks joi, eroman@: can you help me fix this?

@skylined: if you're not actively debugging and pushing this forward, could you find a new owner?

Sure, I'll try to get ClusterFuzz to help me pin down the change that introduced this:
https://cluster-fuzz.appspot.com/testcase?key=19176760

@skylined: this seems to have gone two weeks without update. How is the fix progressing?

ClusterFuzz can't reproduce.

@wtc/eroman: can you help me fix this or find an owner?

@scarybeasts: Not.

I can't reproduce it too. Tested on dev 19.0.1041.0 (121843) and canary 19.0.1049.1 (123065).

@slaweck: it's possible this was some transient regression that got taken care of shortly after it was introduced. If you're satisfied that recent builds seem ok, and we can close this out.

Last time I saw it in dev 18.0.1025.7 (120697).
Yes, I think You can close this out.

Heisenbug! Bah.

This bug has already been fixed! See  issue 112983 

This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Bulk update: removing view restriction from closed bugs.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot