Comment 6 for bug 1298314

Revision history for this message
Charles Kerr (charlesk) wrote :

The crash looks to be happening in this part of signUrl.

  > argc = oauth_split_url_parameters(url.toUtf8().data(), &argv);
  > // Fixup the URL as liboauth is escaping '+' to ' ' in it, incorrectly.
  > for (int a = 0; argv[0][a] != 0; a++)
  > argv[0][a] = argv[0][a] == ' ' ? '+' : argv[0][a];

Where url is coming from this call:

  > QString authHeader = token.signUrl(impl->downloadUrl, QStringLiteral("HEAD"));

and the stacktrace's only mention of downloadUrl is

  > downloadUrl\000

So the fix of the /immediate/ crash is probably to sanity check argc>0 before walking argv[0] to unescape the '+' characters, eg:

  > argc = oauth_split_url_parameters(url.toUtf8().data(), &argv);
  > // Fixup the URL as liboauth is escaping '+' to ' ' in it, incorrectly.
  > if (argc > 0)
  > for (int a = 0; argv[0][a] != 0; a++)
  > argv[0][a] = argv[0][a] == ' ' ? '+' : argv[0][a];

Although that's a worthwhile safeguard anyway to avoid a crash, it's not a full fix -- program would then return a QString "OAuth " which click::DownloadManager::fetchClickToken() will set as a QNetworkRequest's "Authorization" header's value, and then set its url to impl->downloadUrl, which is still an empty string. So the second part of the fix might be for fetchClickToken() to check impl->downloadUrl before creating a QNetworkRequest.