Page MenuHomePhabricator
Create Task
Maniphest T12867

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

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.

Revisions and Commits

rP Phabricator
D18149rP988a52cf1acd Fix ambiguous URI parsing in Youtube Remarkup rule

Related Objects

Event Timeline

jcarrillo7 created this task.Jun 23 2017, 5:57 AM
Herald added a subscriber: eadler. · View Herald TranscriptJun 23 2017, 5:57 AM
jcarrillo7 added a comment.Jun 23 2017, 6:07 AM

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.

jcarrillo7 added a comment.Jun 23 2017, 6:12 AM

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.

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

How can we reproduce this issue locally?

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

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.

jcarrillo7 added a comment.Jun 23 2017, 6:46 AM

You and me both. I am super confused.

jcarrillo7 added a comment.Jun 23 2017, 6:47 AM

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

jcarrillo7 added a comment.Jun 23 2017, 6:47 AM

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

epriestley claimed this task.Jun 23 2017, 1:35 PM

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.
epriestley added a revision: D18149: Fix ambiguous URI parsing in Youtube Remarkup rule.Jun 23 2017, 2:37 PM
jcarrillo7 added a comment.Jun 23 2017, 3:42 PM

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

epriestley closed this task as Resolved by committing rP988a52cf1acd: Fix ambiguous URI parsing in Youtube Remarkup rule.Jun 23 2017, 3:43 PM
epriestley added a commit: rP988a52cf1acd: Fix ambiguous URI parsing in Youtube Remarkup rule.
epriestley added a comment.Jun 23 2017, 3:43 PM

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.

jcarrillo7 added a comment.Jun 23 2017, 4:47 PM

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

Phabricator · Built by Phacility