Differential D14747

404 older-style Phame URIs properly
ClosedPublic
Actions

Authored by epriestley on Dec 12 2015, 1:28 AM.

Details

Reviewers

chad

Maniphest Tasks

T9968: Phame can fatal if handed an older-style URI

Commits

Restricted Diffusion Commit
rPdbdd70270230: 404 older-style Phame URIs properly

Summary

Ref T9968. Some of the crumb/route handling wasn't quite tight enough and could hit a fatal.

Test Plan

Hit previously-fataling URI, got a 404 instead.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 35666.Dec 12 2015, 1:28 AM

epriestley retitled this revision from to 404 older-style Phame URIs properly.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T9968: Phame can fatal if handed an older-style URI.

chad accepted this revision.Dec 12 2015, 1:30 AM

chad edited edge metadata.

This revision is now accepted and ready to land.Dec 12 2015, 1:30 AM

starruler added a subscriber: starruler.Dec 12 2015, 1:31 AM

starruler added inline comments.

src/applications/phame/controller/PhameLiveController.php
166–173	I can't add a macro to this install so here you go

Closed by commit rPdbdd70270230: 404 older-style Phame URIs properly (authored by epriestley, committed by epriestley). · Explain WhyDec 12 2015, 1:32 AM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in T9968: Phame can fatal if handed an older-style URI.Dec 12 2015, 1:36 AM

joshuaspence added a subscriber: joshuaspence.Dec 12 2015, 2:28 AM

joshuaspence added inline comments.

src/applications/phame/application/PhabricatorPhameApplication.php
43	Is this still valid?
src/applications/phame/controller/PhameLiveController.php
169	Four levels of nesting :(

epriestley added inline comments.Dec 12 2015, 2:31 AM

src/applications/phame/application/PhabricatorPhameApplication.php
43	Yes, I added this this morning.
src/applications/phame/controller/PhameLiveController.php
169	How would you suggest rewriting it?

joshuaspence added inline comments.Dec 12 2015, 2:37 AM

src/applications/phame/application/PhabricatorPhameApplication.php
43	Lol okay, makes sense...
src/applications/phame/controller/PhameLiveController.php
169	Oh I don't have any ideas, just making a remark.

Revision Contents
Changeset List

Path

Size

src/

applications/

phame/

application/

PhabricatorPhameApplication.php

6 lines

controller/

PhameLiveController.php

22 lines

Diff 35667

View Options

src/applications/phame/application/PhabricatorPhameApplication.php