r98614 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r98613‎ | r98614 | r98615 >
Date:08:27, 1 October 2011
Author:petrb
Status:deferred (Comments)
Tags:
Comment:
Created EXAMPLE_TOKEN which replaces all tokens in api examples and also looks like a real token,
I think it save a work of many tool developing people since I myself had troubles understanding
how real token looks from the api.php descriptions (previous example also contained some charactes
which would hardly be inside the string, token is always lower case)
Modified paths:
  • /branches/petrb/phase3/includes/Defines.php (modified) (history)
  • /branches/petrb/phase3/includes/api/ApiDelete.php (modified) (history)
  • /branches/petrb/phase3/includes/api/ApiImport.php (modified) (history)
  • /branches/petrb/phase3/includes/api/ApiMove.php (modified) (history)
  • /branches/petrb/phase3/includes/api/ApiPatrol.php (modified) (history)
  • /branches/petrb/phase3/includes/api/ApiProtect.php (modified) (history)
  • /branches/petrb/phase3/includes/api/ApiRollback.php (modified) (history)
  • /branches/petrb/phase3/includes/api/ApiUndelete.php (modified) (history)

Diff [purge]

Index: branches/petrb/phase3/includes/Defines.php
——@@ -250,3 +250,8 @@
251251 define( 'PROTO_CURRENT', null );
252252 define( 'PROTO_CANONICAL', 1 );
253253 define( 'PROTO_INTERNAL', 2 );
 254+
 255+/**
 256+ * Common example strings for api examples
 257+ */
 258+define( 'EXAMPLE_TOKEN', 'abcdef123456789ab12ab12cd45ef78' ); //in case that way how token looks change it would save us a lot of work rewriting all examples
Index: branches/petrb/phase3/includes/api/ApiMove.php
——@@ -256,7 +256,7 @@
257257
258258 public function getExamples() {
259259 return array(
260 - 'api.php?action=move&from=Exampel&to=Example&token=123ABC&reason=Misspelled%20title&movetalk=&noredirect='
 260+ 'api.php?action=move&from=Exampel&to=Example&token=' . EXAMPLE_TOKEN . '&reason=Misspelled%20title&movetalk=&noredirect='
261261 );
262262 }
263263
Index: branches/petrb/phase3/includes/api/ApiProtect.php
——@@ -219,8 +219,8 @@
220220
221221 public function getExamples() {
222222 return array(
223 - 'api.php?action=protect&title=Main%20Page&token=123ABC&protections=edit=sysop|move=sysop&cascade=&expiry=20070901163000|never',
224 - 'api.php?action=protect&title=Main%20Page&token=123ABC&protections=edit=all|move=all&reason=Lifting%20restrictions'
 223+ 'api.php?action=protect&title=Main%20Page&token=' . EXAMPLE_TOKEN . '&protections=edit=sysop|move=sysop&cascade=&expiry=20070901163000|never',
 224+ 'api.php?action=protect&title=Main%20Page&token=' . EXAMPLE_TOKEN . '&protections=edit=all|move=all&reason=Lifting%20restrictions'
225225 );
226226 }
227227
Index: branches/petrb/phase3/includes/api/ApiRollback.php
——@@ -186,8 +186,8 @@
187187
188188 public function getExamples() {
189189 return array(
190 - 'api.php?action=rollback&title=Main%20Page&user=Catrope&token=123ABC',
191 - 'api.php?action=rollback&title=Main%20Page&user=217.121.114.116&token=123ABC&summary=Reverting%20vandalism&markbot=1'
 190+ 'api.php?action=rollback&title=Main%20Page&user=Catrope&token=' . EXAMPLE_TOKEN,
 191+ 'api.php?action=rollback&title=Main%20Page&user=217.121.114.116&token=' . EXAMPLE_TOKEN . '&summary=Reverting%20vandalism&markbot=1'
192192 );
193193 }
194194
Index: branches/petrb/phase3/includes/api/ApiDelete.php
——@@ -271,8 +271,8 @@
272272
273273 public function getExamples() {
274274 return array(
275 - 'api.php?action=delete&title=Main%20Page&token=123ABC',
276 - 'api.php?action=delete&title=Main%20Page&token=123ABC&reason=Preparing%20for%20move'
 275+ 'api.php?action=delete&title=Main%20Page&token=' . EXAMPLE_TOKEN,
 276+ 'api.php?action=delete&title=Main%20Page&token' . EXAMPLE_TOKEN . '&reason=Preparing%20for%20move'
277277 );
278278 }
279279
Index: branches/petrb/phase3/includes/api/ApiImport.php
——@@ -161,7 +161,7 @@
162162 public function getExamples() {
163163 return array(
164164 'Import [[meta:Help:Parserfunctions]] to namespace 100 with full history:',
165 - ' api.php?action=import&interwikisource=meta&interwikipage=Help:ParserFunctions&namespace=100&fullhistory=&token=123ABC',
 165+ ' api.php?action=import&interwikisource=meta&interwikipage=Help:ParserFunctions&namespace=100&fullhistory=&token=' . EXAMPLE_TOKEN,
166166 );
167167 }
168168
Index: branches/petrb/phase3/includes/api/ApiPatrol.php
——@@ -106,7 +106,7 @@
107107
108108 public function getExamples() {
109109 return array(
110 - 'api.php?action=patrol&token=123abc&rcid=230672766'
 110+ 'api.php?action=patrol&token=' . EXAMPLE_TOKEN . '&rcid=230672766'
111111 );
112112 }
113113
Index: branches/petrb/phase3/includes/api/ApiUndelete.php
——@@ -154,8 +154,8 @@
155155
156156 public function getExamples() {
157157 return array(
158 - 'api.php?action=undelete&title=Main%20Page&token=123ABC&reason=Restoring%20main%20page',
159 - 'api.php?action=undelete&title=Main%20Page&token=123ABC&timestamps=20070703220045|20070702194856'
 158+ 'api.php?action=undelete&title=Main%20Page&token=' . EXAMPLE_TOKEN . '&reason=Restoring%20main%20page',
 159+ 'api.php?action=undelete&title=Main%20Page&token=token=' . EXAMPLE_TOKEN . '&timestamps=20070703220045|20070702194856'
160160 );
161161 }
162162

Follow-up revisions

RevisionCommit summaryAuthorDate
r98615double variable id, fixpetrb08:33, 1 October 2011
r98740Fixed missing "=" in ApiDelete.phppetrb06:11, 3 October 2011

Comments

#Comment by Catrope (talk | contribs)   08:31, 1 October 2011

In fact, edit tokens also have +\ appended to them, which clients need to urlencode as %2B%5C

#Comment by Petrb (talk | contribs)   08:31, 1 October 2011

I didn't change edit tokens, only 123ABC ones

#Comment by Catrope (talk | contribs)   08:34, 1 October 2011

Even those tokens have +\, don't they?

#Comment by Petrb (talk | contribs)   08:36, 1 October 2011

I don't think so since I do not append it there in huggle and it works

#Comment by Petrb (talk | contribs)   08:37, 1 October 2011

anyway it's no problem now to update it since it's all in one file :)

#Comment by Catrope (talk | contribs)   08:38, 1 October 2011

You don't have to append it yourself. I guess my wording wasn't very clear. Tokens consist of 32 lowercase hexadecimal characters, followed by a plus and a backslash. Or as a regex: /[0-9a-f]{32}\+\\/ . So if you want example tokens to be realistic, you should also include the +\ part.

#Comment by Petrb (talk | contribs)   08:40, 1 October 2011

I meant that in huggle we parse it with /[0-9][A-Z][a-z]/ regex and we do not append +\, but I checked login api only so far, maybe in other cases it's different, but if it works for login, maybe it works everywhere, anyway if you are sure, I can update it here

#Comment by Catrope (talk | contribs)   08:44, 1 October 2011

Hah, you're right. Login tokens don't have +\ . Looking at the code it seems we don't reuse the edit token generation function there (which we do use everywhere else) because we explicitly don't want to reuse any edit token that might still be in the session.

Pretty much all other tokens will have +\ though.

#Comment by Petrb (talk | contribs)   08:48, 1 October 2011

right. if it's not going to change and login token will not contain it in future, I will update EXAMPLE_TOKEN and remove it from that one example where it's different, anyway I think tokens should be same everywhere because tool dev need to make more token parsing functions in case that mw produces more different kinds of token

#Comment by Catrope (talk | contribs)   10:22, 1 October 2011

Clients shouldn't parse or validate tokens. They should make no assumptions about what tokens look like, but just store whatever the API gives them when they ask for a token, and pass it back unmodified.

That said, I see no reason why the login token would have to change in the foreseeable future.

#Comment by Johnduhart (talk | contribs)   16:27, 1 October 2011

+1 what roan said, your application not be parsing tokens. If tomorrow we changed all tokens to be R33DYRULEZ your application should not bat an eye about it. Just take what the API gives you and give it back.

#Comment by Petrb (talk | contribs)   16:30, 1 October 2011

ok, ok... I understood since the first reply, anyway I already commited the change so that now it follows what he noticed

#Comment by Krinkle (talk | contribs)   17:29, 2 October 2011 
+			'api.php?action=delete&title=Main%20Page&token' . EXAMPLE_TOKEN . '&reason=Preparing%20for%20move'
...
+			'api.php?action=undelete&title=Main%20Page&token=token=' . EXAMPLE_TOKEN . '&timestamps=20070703220045|20070702194856'

These two are broken.

Status & tagging log