MediaWiki r54127 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r54126‎ | r54127 (on ViewVC)‎ | r54128 >
Date:21:56, 31 July 2009
Author:mrzman
Status:resolved (Comments)
Tags:
Comment:
(bug 19907) Adds support for cross-domain AJAX requests to the API.
Uses the Access-Control-Allow-Origin header for browsers that support it.
<http://dev.w3.org/2006/waf/access-control/&gt;
$wgCrossSiteAJAXdomains can be set to '*' to allow requests from any domain,
an array of domains to allow, or, if $wgCrossSiteAJAXdomainsRegex is true,
an array of regexes to match against the request origin
Modified paths:

Diff [purge]

Loading diff…

Follow-up revisions

Rev.Commit summaryAuthorDate
r55400Tweak Access-Control-Allow-Origin stuff per comments on r54127....mrzman00:22, 21 August 2009

Comments

#Comment by Brion VIBBER (talk | contribs)   00:21, 20 August 2009

Hmmm... couple questions:

  • Should we be validating the http-origin header value before spitting it back to output?
  • Domains are normally case-insensitive, but this is doing case-sensitive matches
  • I think I'd rather use regexes or wildcards consistently rather than a switch that forces you to go back and change all your existing rules when you change it. (wgCrossSiteAJAXdomainsRegex)
#Comment by Mr.Z-man (talk | contribs)   00:41, 20 August 2009

With the exception of the regex part, I was going by the spec draft, which specifies a case-insensitive match.

If the value of the Origin header is not a case-sensitive match for any of the values in list of origins do not set any additional headers and terminate this set of steps.

For the majority of end users, I figured it would be simplest to just use a list of domains, I mainly added the regex option so if someone wanted to allow, say, all Wikimedia sites, they wouldn't need a list of 800 domains. Would you suggest just always using regexes, or just having 2 arrays?

What kind of validation are you thinking of?

#Comment by Brion VIBBER (talk | contribs)   20:52, 20 August 2009

Ok, case-sensitive is indeed per spec here. :) That's very weird IMO, but if that's what they say, hey that's what they specify. :)

I was thinking wildcards might be easier to work with than regexes. For instance:

$wgCrossSiteAJAXdomains = array(
  'www.mediawiki.org',
  '*.wikipedia.org',
  '*.wikimedia.org',
  '*.wiktionary.org',
 );

This wouldn't require any manual switching in of the wildcard mode; you can add a wildcard into an existing list of straight domains without having to go back and change all the other entries to a different format.

It might also be useful to allow for exceptions... either inline in the same array:

$wgCrossSiteAJAXdomains = array(
  '*.wikipedia.org',
  '*.wikimedia.org',
  '!upload.wikimedia.org',
 );

or separately:

$wgCrossSiteAJAXdomains = array(
  '*.wikipedia.org',
  '*.wikimedia.org',
  '!upload.wikimedia.org',
 );
$wgCrossSiteAJAXdomainExceptions = array(
  'upload.wikimedia.org',
 );
#Comment by Mr.Z-man (talk | contribs)   00:23, 21 August 2009

Wildcard support added in r55400

Status & tagging log