Page MenuHomePhabricator

Youtube remarkup rule fails to parse "ambiguous URI"
Closed, ResolvedPublic

Description

Saw this error today while importing a new repository (Note I have replaced some sensitive logs with XXX):

Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700] [2017-06-22 22:50:05] EXCEPTION: (PhutilProxyException) Error while executing Task ID 2828587. {>} (Exception) Rejecting ambiguous URI "XXX://123456 XXX XXX? XXX://123456 XXX XXX". This URI is not formatted or encoded properly. at [<phutil>/src/parser/PhutilURI.php:51]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700] arcanist(head=master, ref.master=c04f141ab023),phab-extensions(head=master, ref.master=68d7cc5707dc), phabricator(head=master, ref.master=58df1b7d3be2), phutil(head=master, ref.master=fa993854745c)
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #0 <#2> PhutilURI::__construct(string) called at [<phabricator>/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php:12]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #1 <#2> PhabricatorYoutubeRemarkupRule::apply(string) called at [<phutil>/src/markup/engine/remarkup/blockrule/PhutilRemarkupBlockRule.php:84]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #2 <#2> PhutilRemarkupBlockRule::applyRules(string) called at [<phutil>/src/markup/engine/remarkup/blockrule/PhutilRemarkupDefaultBlockRule.php:17]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #3 <#2> PhutilRemarkupDefaultBlockRule::markupText(string, NULL) called at [<phutil>/src/markup/engine/PhutilRemarkupEngine.php:232]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #4 <#2> PhutilRemarkupEngine::markupBlock(array) called at [<phutil>/src/markup/engine/PhutilRemarkupEngine.php:128]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #5 <#2> PhutilRemarkupEngine::preprocessText(string) called at [<phutil>/src/markup/engine/PhutilRemarkupEngine.php:94]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #6 <#2> PhutilRemarkupEngine::markupText(string) called at [<phabricator>/src/infrastructure/markup/PhabricatorMarkupEngine.php:602]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #7 <#2> PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles(PhabricatorUser, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:3245]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #8 <#2> PhabricatorApplicationTransactionEditor::extractFilePHIDs(PhabricatorRepositoryCommit, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:959]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #9 <#2> PhabricatorApplicationTransactionEditor::applyTransactions(PhabricatorRepositoryCommit, array) called at [<phabricator>/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php:102]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #10 <#2> PhabricatorRepositoryCommitHeraldWorker::parseCommit(PhabricatorRepository, PhabricatorRepositoryCommit) called at [<phabricator>/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php:51]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #11 <#2> PhabricatorRepositoryCommitParserWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:123]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #12 <#2> PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php:171]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #13 <#2> PhabricatorWorkerActiveTask::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php:22]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #14 PhabricatorTaskmasterDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:219]
Daemon 87860 STDE [Thu, 22 Jun 2017 22:50:05 -0700]   #15 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:131]
Daemon 87860 FAIL [Thu, 22 Jun 2017 22:50:05 -0700] Process exited with error 255.

This causes the repository to sit at importing 99.99%. It also causes any attempt to view that commit in the repo to show "Unhandled Exception.... This URI is not formatted or encoded properly." in the UI.

Event Timeline

The URI that is ambiguous is a URI to a company App on MacOS. The only part of the URI that matters is the xxx://123456. The rest is just the title of the item referenced by this URI and this title contains a "?" which mixed with T12526 may be causing this issue. There may also be a place you now need to catch exceptions thanks to URI parsing logic changes. Just guessing here from what I can tell in the code.

There also seems like there may be an issue with the current parsing logic since the "detected URI" has spaces which I do not think are valid in a uri and it should have been detected as two separate URI's with some stuff in the middle.

chad added a subscriber: chad.Jun 23 2017, 6:18 AM

How can we reproduce this issue locally?

I'm not sure how without giving you the entire commit message which I cannot. I think key would be a having a commit with a line (probably the first one) that looks like this "XXX://123456 XXX XXX XXX? XXX://123456 XXX XXX XXX"

Not sure how I can help with repro steps but do let me know if I can provide any other logs or useful things.

jcarrillo7 added a comment.EditedJun 23 2017, 6:42 AM

LOL @chad Literally reproduced this issue here by trying to paste the above line without the back ticks. It refuses to let me comment.

chad added a comment.EditedJun 23 2017, 6:44 AM

Well the stack trace says PhabricatorYoutubeRemarkupRule, so I'm confused what the issue is.

You and me both. I am super confused.

"XXX://123456 XXX XXX XXX://123456 XXX XXX"

Looks to be just the presence of the "?" in the text

I think a minimal reproduction case which is typical of this example is:

A://?:

This requires an upstream fix. After we fix it and you upgrade, the task should finish normally. Fix should land today.

There are some workarounds to get this repository out of the parsing phase right away if this is urgent. Let me know if you'd like me to walk you through that stuff.

epriestley renamed this task from Daemon fails to parse "ambiguous URI" to Youtube remarkup rule fails to parse "ambiguous URI".Jun 23 2017, 2:30 PM
epriestley removed projects: Diffusion, Daemons.

I’m fine waiting for the fix. Thanks @epriestley

This should now be fixed in HEAD of master, and promote to stable within about 12 hours. Thanks for the report! Let us know if you run into anything else.

I can verify this fixes it! Thanks for this guys.