Parsoid: Redirects should not be served as HTTP 200 with contents of target
Closed, ResolvedPublic

Description

When editing page with redirect, I can edit just the page to where it is redirected.


Version: unspecified
Severity: normal

bzimport added a project: Parsoid.Via ConduitNov 22 2014, 1:24 AM
bzimport set Reference to bz45808.
Juandev created this task.Via LegacyMar 6 2013, 10:26 PM
Jdforrester-WMF added a comment.Via ConduitMar 8 2013, 4:36 PM

Juan,

Thanks for your bug report; you are correct. This is because Parsoid gives the wrong response for pages with redirects. Pushing over to them.

Current behaviour:

  • 200 response with the contents of the target page

On what we'd expect, I'd suggest either type A (but it's messy architecturally) or type B (but not sure about user agent support for HTTP 300):

Expected behaviour - type A:

  • when in 'reading mode', a 302 to the page with the redirect;
  • when in 'editing mode', a 200 with the contents of the page (to edit the redirect / etc.)

Expected behaviour - type B:

  • a 300 response with the first link (for regular browsers) being off to the target of the page, and the second being the ?redirect=no or whatever version to be able to edit it.
cscott added a comment.Via ConduitApr 17 2013, 5:51 PM

If you ask Parsoid for (say) http://localhost:8000/en/OLPC you get the page for "One Laptop Per Child" (the <title> is correct), but the returned page contains {{Redirect|OLPC}} in addition to the text from the "One Laptop Per Child" article.

So in addition to the redirect being transparent, we're also getting bogus extra content in the parsed version.

cscott added a comment.Via ConduitApr 17 2013, 6:02 PM

From IRC discussion w/ GWicke:

gwicke: I think that we should not resolve redirects for top-level requests
gwicke: should return a meta instead that lets the VE edit the redirect
cscott-free: yeah, although editing "#REDIRECT [[foo]]" is a bit awkward, that's an <ol>
gwicke: obviously we'd expose it as <meta property="mw:Redirect" content="foo"> or something like that
cscott-free: i think i learned recently that there can be other 'stuff' in a redirect?
gwicke: arguably this should eventually end up in the head instead of the content
gwicke: but for now we should probably keep it in the content area in the interest of round-tripping
cscott-free: that makes sense.
gwicke: cscott: there can be page content following it that is normally ignored by MW
gwicke: but we should probably still round-trip it
gwicke: hence the idea to keep it inline for now rather than returning a blank body with meta info in the head
(01:58:07 PM) cscott-free: i suspect there should be a corresponding VE bug for editing the <meta property="mw:Redirect" content="foo">
(01:58:08 PM) gwicke: maybe we should use link / rel / href again
(01:58:21 PM) gwicke: for the same relative link resolution reasons

(see bug 45206 comment 5 for a brief discussion of the meta/link issues)

cscott added a comment.Via ConduitApr 17 2013, 6:27 PM

See bug 47328 for the corresponding Visual Editor bug.

cscott added a comment.Via ConduitApr 30 2013, 7:34 PM

Note: test "#REDIRECT [[www.google.com]]" which currently does unusual things.

GWicke added a comment.Via ConduitApr 30 2013, 7:59 PM

"#REDIRECT [[www.google.com]]" behaves just the same way as "#REDIRECT [[Foo]]", so not that unexpected.

gerritbot added a comment.Via ConduitMay 1 2013, 5:35 PM

Related URL: https://gerrit.wikimedia.org/r/61799 (Gerrit Change I1bd36f32a5e46b90261895e5499a0308875e5e05)

GWicke added a comment.Via ConduitMay 14 2013, 9:54 PM
  • Bug 48271 has been marked as a duplicate of this bug. ***
Jdforrester-WMF added a comment.Via ConduitMay 17 2013, 6:19 AM

As the gerrit commit is merged, can this now be closed?

GWicke added a comment.Via ConduitMay 17 2013, 6:21 AM

I have not tested the HTTP responses yet, but am happy to close. Please reopen if you find issues (or open a new bug).

cscott added a comment.Via ConduitMay 17 2013, 7:42 AM

Sorry: the wikitext parsing bit landed but I didn't yet make any changes to how the server handles redirects. One more patch is needed to change the http status and turn off the old redirect code. Reopening.

GWicke added a comment.Via ConduitJun 12 2013, 12:28 AM

I removed our previous behavior of following redirects when encountering a redirect page, so we now return the regular redirect content as expected. Closing as fixed.

Aklapper added a comment.Via ConduitJul 4 2013, 10:35 AM

[Parsoid component reorg by merging JS/General and General. See bug 50685 for more information. Filter bugmail on this comment. parsoidreorg20130704]

Add Comment