r24092 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r24091‎ | r24092 | r24093 >
Date:19:04, 14 July 2007
Author:yurik
Status:old
Tags:
Comment:
API: Big change: Removed all userCanRead() checks per IRC discussion. Only rvprop=content will now check that the page can be read.
Modified paths:
  • /trunk/phase3/includes/api/ApiMain.php (modified) (history)
  • /trunk/phase3/includes/api/ApiPageSet.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQuery.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryAllLinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryAllpages.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryBacklinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryBase.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryCategories.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryCategoryMembers.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryExtLinksUsage.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryImages.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryLinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryLogEvents.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryRecentChanges.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryRevisions.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryUserContributions.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryWatchlist.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiQueryRecentChanges.php
——@@ -125,23 +125,12 @@
126126 $result->addValue('query', $this->getModuleName(), $data);
127127 }
128128
129 - /**
130 - * Security overview: As implemented, any change to a restricted page (userCanRead() == false)
131 - * is hidden from the client, except when a page is being moved to a non-restricted name,
132 - * or when a non-restricted becomes restricted. When shown, all other fields are shown as well.
133 - */
134129 private function extractRowInfo($row) {
135 - $title = Title :: makeTitle($row->rc_namespace, $row->rc_title);
136130 $movedToTitle = false;
137131 if (!empty($row->rc_moved_to_title))
138132 $movedToTitle = Title :: makeTitle($row->rc_moved_to_ns, $row->rc_moved_to_title);
139133
140 - // If either this is an edit of a restricted page,
141 - // or a move where both to and from names are restricted, skip
142 - if (!$title->userCanRead() && (!$movedToTitle ||
143 - ($movedToTitle && !$movedToTitle->userCanRead())))
144 - return false;
145 -
 134+ $title = Title :: makeTitle($row->rc_namespace, $row->rc_title);
146135 $vals = array ();
147136
148137 $vals['type'] = intval($row->rc_type);
——@@ -149,7 +138,7 @@
150139 if ($this->fld_title) {
151140 ApiQueryBase :: addTitleInfo($vals, $title);
152141 if ($movedToTitle)
153 - ApiQueryBase :: addTitleInfo($vals, $movedToTitle, false, "new_");
 142+ ApiQueryBase :: addTitleInfo($vals, $movedToTitle, "new_");
154143 }
155144
156145 if ($this->fld_ids) {
Index: trunk/phase3/includes/api/ApiQueryAllLinks.php
——@@ -101,17 +101,15 @@
102102 }
103103
104104 if (is_null($resultPageSet)) {
105 - $title = Title :: makeTitle($row->pl_namespace, $row->pl_title);
106 - if ($title->userCanRead()) {
107 - $vals = array();
108 - if ($fld_ids)
109 - $vals['fromid'] = intval($row->pl_from);
110 - if ($fld_title) {
111 - $vals['ns'] = intval($title->getNamespace());
112 - $vals['title'] = $title->getPrefixedText();
113 - }
114 - $data[] = $vals;
 105+ $vals = array();
 106+ if ($fld_ids)
 107+ $vals['fromid'] = intval($row->pl_from);
 108+ if ($fld_title) {
 109+ $title = Title :: makeTitle($row->pl_namespace, $row->pl_title);
 110+ $vals['ns'] = intval($title->getNamespace());
 111+ $vals['title'] = $title->getPrefixedText();
115112 }
 113+ $data[] = $vals;
116114 } else {
117115 $pageids[] = $row->pl_from;
118116 }
Index: trunk/phase3/includes/api/ApiQuery.php
——@@ -293,7 +293,7 @@
294294 // Report any missing titles
295295 foreach ($pageSet->getMissingTitles() as $fakeId => $title) {
296296 $vals = array();
297 - ApiQueryBase :: addTitleInfo($vals, $title, true);
 297+ ApiQueryBase :: addTitleInfo($vals, $title);
298298 $vals['missing'] = '';
299299 $pages[$fakeId] = $vals;
300300 }
——@@ -310,7 +310,7 @@
311311 foreach ($pageSet->getGoodTitles() as $pageid => $title) {
312312 $vals = array();
313313 $vals['pageid'] = $pageid;
314 - ApiQueryBase :: addTitleInfo($vals, $title, true);
 314+ ApiQueryBase :: addTitleInfo($vals, $title);
315315 $pages[$pageid] = $vals;
316316 }
317317
Index: trunk/phase3/includes/api/ApiQueryLinks.php
——@@ -102,9 +102,8 @@
103103 $lastId = $row->pl_from;
104104 }
105105
106 - $title = Title :: makeTitle($row->pl_namespace, $row->pl_title);
107106 $vals = array();
108 - ApiQueryBase :: addTitleInfo($vals, $title, true);
 107+ ApiQueryBase :: addTitleInfo($vals, Title :: makeTitle($row->pl_namespace, $row->pl_title));
109108 $data[] = $vals;
110109 }
111110
——@@ -116,9 +115,7 @@
117116
118117 $titles = array();
119118 while ($row = $db->fetchObject($res)) {
120 - $title = Title :: makeTitle($row->pl_namespace, $row->pl_title);
121 - if($title->userCanRead())
122 - $titles[] = $title;
 119+ $titles[] = Title :: makeTitle($row->pl_namespace, $row->pl_title);
123120 }
124121 $resultPageSet->populateFromTitles($titles);
125122 }
Index: trunk/phase3/includes/api/ApiQueryExtLinksUsage.php
——@@ -110,19 +110,17 @@
111111 }
112112
113113 if (is_null($resultPageSet)) {
114 - $title = Title :: makeTitle($row->page_namespace, $row->page_title);
115 - if ($title->userCanRead()) {
116 - $vals = array();
117 - if ($fld_ids)
118 - $vals['pageid'] = intval($row->page_id);
119 - if ($fld_title) {
120 - $vals['ns'] = intval($title->getNamespace());
121 - $vals['title'] = $title->getPrefixedText();
122 - }
123 - if ($fld_url)
124 - $vals['url'] = $row->el_to;
125 - $data[] = $vals;
 114+ $vals = array();
 115+ if ($fld_ids)
 116+ $vals['pageid'] = intval($row->page_id);
 117+ if ($fld_title) {
 118+ $title = Title :: makeTitle($row->page_namespace, $row->page_title);
 119+ $vals['ns'] = intval($title->getNamespace());
 120+ $vals['title'] = $title->getPrefixedText();
126121 }
 122+ if ($fld_url)
 123+ $vals['url'] = $row->el_to;
 124+ $data[] = $vals;
127125 } else {
128126 $resultPageSet->processDbRow($row);
129127 }
——@@ -197,7 +195,7 @@
198196 }
199197
200198 public function getVersion() {
201 - return __CLASS__ . ': $Id:$';
 199+ return __CLASS__ . ': $Id$';
202200 }
203201 }
204202 ?>
Index: trunk/phase3/includes/api/ApiQueryAllpages.php
——@@ -94,12 +94,10 @@
9595
9696 if (is_null($resultPageSet)) {
9797 $title = Title :: makeTitle($row->page_namespace, $row->page_title);
98 - if ($title->userCanRead()) {
99 - $data[] = array(
100 - 'pageid' => intval($row->page_id),
101 - 'ns' => intval($title->getNamespace()),
102 - 'title' => $title->getPrefixedText());
103 - }
 98+ $data[] = array(
 99+ 'pageid' => intval($row->page_id),
 100+ 'ns' => intval($title->getNamespace()),
 101+ 'title' => $title->getPrefixedText());
104102 } else {
105103 $resultPageSet->processDbRow($row);
106104 }
Index: trunk/phase3/includes/api/ApiMain.php
——@@ -96,8 +96,8 @@
9797
9898 // Impose module restrictions.
9999 // If the current user cannot read,
100 - // Remove all modules other than login & help
101 - global $wgUser, $wgWhitelistRead;
 100+ // Remove all modules other than login
 101+ global $wgUser;
102102 if (!$wgUser->isAllowed('read')) {
103103 self::$Modules = array(
104104 'login' => self::$Modules['login'],
Index: trunk/phase3/includes/api/ApiQueryBacklinks.php
——@@ -186,13 +186,9 @@
187187
188188 private function extractRowInfo($row) {
189189
190 - $title = Title :: makeTitle($row->page_namespace, $row->page_title);
191 - if (!$title->userCanRead())
192 - return false;
193 -
194190 $vals = array();
195191 $vals['pageid'] = intval($row->page_id);
196 - ApiQueryBase :: addTitleInfo($vals, $title);
 192+ ApiQueryBase :: addTitleInfo($vals, Title :: makeTitle($row->page_namespace, $row->page_title));
197193
198194 return $vals;
199195 }
——@@ -239,7 +235,7 @@
240236 $rootNs = intval($continueList[0]);
241237 if (($rootNs !== 0 || $continueList[0] === '0') && !empty ($continueList[1])) {
242238 $this->rootTitle = Title :: makeTitleSafe($rootNs, $continueList[1]);
243 - if ($this->rootTitle && $this->rootTitle->userCanRead()) {
 239+ if ($this->rootTitle) {
244240
245241 $step = intval($continueList[2]);
246242 if ($step === 1 || $step === 2) {
——@@ -287,7 +283,7 @@
288284 $rootNs = intval($continueList[0]);
289285 if (($rootNs !== 0 || $continueList[0] === '0') && !empty ($continueList[1])) {
290286 $this->rootTitle = Title :: makeTitleSafe($rootNs, $continueList[1]);
291 - if ($this->rootTitle && $this->rootTitle->userCanRead()) {
 287+ if ($this->rootTitle) {
292288
293289 $contID = intval($continueList[2]);
294290 if ($contID !== 0) {
Index: trunk/phase3/includes/api/ApiQueryWatchlist.php
——@@ -157,14 +157,10 @@
158158 if ($vals)
159159 $data[] = $vals;
160160 } else {
161 - $title = Title :: makeTitle($row->rc_namespace, $row->rc_title);
162 - // skip any pages that user has no rights to read
163 - if ($title->userCanRead()) {
164 - if ($allrev) {
165 - $data[] = intval($row->rc_this_oldid);
166 - } else {
167 - $data[] = intval($row->rc_cur_id);
168 - }
 161+ if ($allrev) {
 162+ $data[] = intval($row->rc_this_oldid);
 163+ } else {
 164+ $data[] = intval($row->rc_cur_id);
169165 }
170166 }
171167 }
——@@ -184,10 +180,6 @@
185181
186182 private function extractRowInfo($row) {
187183
188 - $title = Title :: makeTitle($row->rc_namespace, $row->rc_title);
189 - if (!$title->userCanRead())
190 - return false;
191 -
192184 $vals = array ();
193185
194186 if ($this->fld_ids) {
——@@ -196,7 +188,7 @@
197189 }
198190
199191 if ($this->fld_title)
200 - ApiQueryBase :: addTitleInfo($vals, $title);
 192+ ApiQueryBase :: addTitleInfo($vals, Title :: makeTitle($row->rc_namespace, $row->rc_title));
201193
202194 if ($this->fld_user) {
203195 $vals['user'] = $row->rc_user_text;
Index: trunk/phase3/includes/api/ApiPageSet.php
——@@ -297,22 +297,18 @@
298298 // Store Title object in various data structures
299299 $title = Title :: makeTitle($row->page_namespace, $row->page_title);
300300
301 - // skip any pages that user has no rights to read
302 - if ($title->userCanRead()) {
 301+ $pageId = intval($row->page_id);
 302+ $this->mAllPages[$row->page_namespace][$row->page_title] = $pageId;
 303+ $this->mTitles[] = $title;
303304
304 - $pageId = intval($row->page_id);
305 - $this->mAllPages[$row->page_namespace][$row->page_title] = $pageId;
306 - $this->mTitles[] = $title;
307 -
308 - if ($this->mResolveRedirects && $row->page_is_redirect == '1') {
309 - $this->mPendingRedirectIDs[$pageId] = $title;
310 - } else {
311 - $this->mGoodTitles[$pageId] = $title;
312 - }
313 -
314 - foreach ($this->mRequestedPageFields as $fieldName => & $fieldValues)
315 - $fieldValues[$pageId] = $row-> $fieldName;
 305+ if ($this->mResolveRedirects && $row->page_is_redirect == '1') {
 306+ $this->mPendingRedirectIDs[$pageId] = $title;
 307+ } else {
 308+ $this->mGoodTitles[$pageId] = $title;
316309 }
 310+
 311+ foreach ($this->mRequestedPageFields as $fieldName => & $fieldValues)
 312+ $fieldValues[$pageId] = $row-> $fieldName;
317313 }
318314
319315 public function finishPageSetGeneration() {
——@@ -595,8 +591,6 @@
596592 // Validation
597593 if ($titleObj->getNamespace() < 0)
598594 $this->dieUsage("No support for special page $titleString has been implemented", 'unsupportednamespace');
599 - if (!$titleObj->userCanRead())
600 - $this->dieUsage("No read permission for $titleString", 'titleaccessdenied');
601595
602596 $linkBatch->addObj($titleObj);
603597 }
Index: trunk/phase3/includes/api/ApiQueryBase.php
——@@ -128,13 +128,9 @@
129129 return $res;
130130 }
131131
132 - public static function addTitleInfo(&$arr, $title, $includeRestricted=false, $prefix='') {
133 - if ($includeRestricted || $title->userCanRead()) {
134 - $arr[$prefix . 'ns'] = intval($title->getNamespace());
135 - $arr[$prefix . 'title'] = $title->getPrefixedText();
136 - }
137 - if (!$title->userCanRead())
138 - $arr[$prefix . 'inaccessible'] = "";
 132+ public static function addTitleInfo(&$arr, $title, $prefix='') {
 133+ $arr[$prefix . 'ns'] = intval($title->getNamespace());
 134+ $arr[$prefix . 'title'] = $title->getPrefixedText();
139135 }
140136
141137 /**
Index: trunk/phase3/includes/api/ApiQueryRevisions.php
——@@ -90,6 +90,15 @@
9191 $this->fld_user = true;
9292 }
9393 if (isset ($prop['content'])) {
 94+
 95+ // For each page we will request, the user must have read rights for that page
 96+ foreach ($pageSet->getGoodTitles() as $title) {
 97+ if( !$title->userCanRead() )
 98+ $this->dieUsage(
 99+ 'The current user is not allowed to read ' . $title->getPrefixedText(),
 100+ 'accessdenied');
 101+ }
 102+
94103 $this->addTables('text');
95104 $this->addWhere('rev_text_id=old_id');
96105 $this->addFields('old_id');
——@@ -132,7 +141,7 @@
133142
134143 // There is only one ID, use it
135144 $this->addWhereFld('rev_page', current(array_keys($pageSet->getGoodTitles())));
136 -
 145+
137146 if(!is_null($user)) {
138147 $this->addWhereFld('rev_user_text', $user);
139148 } elseif (!is_null( $excludeuser)) {
Index: trunk/phase3/includes/api/ApiQueryCategories.php
——@@ -95,11 +95,9 @@
9696 }
9797
9898 $title = Title :: makeTitle(NS_CATEGORY, $row->cl_to);
99 - // do not check userCanRead() -- page content is already accessible,
100 - // and category is listed there.
10199
102100 $vals = array();
103 - ApiQueryBase :: addTitleInfo($vals, $title, true);
 101+ ApiQueryBase :: addTitleInfo($vals, $title);
104102 if ($fld_sortkey)
105103 $vals['sortkey'] = $row->cl_sortkey;
106104
——@@ -114,9 +112,7 @@
115113
116114 $titles = array();
117115 while ($row = $db->fetchObject($res)) {
118 - $title = Title :: makeTitle(NS_CATEGORY, $row->cl_to);
119 - if($title->userCanRead())
120 - $titles[] = $title;
 116+ $titles[] = Title :: makeTitle(NS_CATEGORY, $row->cl_to);
121117 }
122118 $resultPageSet->populateFromTitles($titles);
123119 }
Index: trunk/phase3/includes/api/ApiQueryCategoryMembers.php
——@@ -100,19 +100,17 @@
101101 $lastSortKey = $row->cl_sortkey; // detect duplicate sortkeys
102102
103103 if (is_null($resultPageSet)) {
104 - $title = Title :: makeTitle($row->page_namespace, $row->page_title);
105 - if ($title->userCanRead()) {
106 - $vals = array();
107 - if ($fld_ids)
108 - $vals['pageid'] = intval($row->page_id);
109 - if ($fld_title) {
110 - $vals['ns'] = intval($title->getNamespace());
111 - $vals['title'] = $title->getPrefixedText();
112 - }
113 - if ($fld_sortkey)
114 - $vals['sortkey'] = $row->cl_sortkey;
115 - $data[] = $vals;
 104+ $vals = array();
 105+ if ($fld_ids)
 106+ $vals['pageid'] = intval($row->page_id);
 107+ if ($fld_title) {
 108+ $title = Title :: makeTitle($row->page_namespace, $row->page_title);
 109+ $vals['ns'] = intval($title->getNamespace());
 110+ $vals['title'] = $title->getPrefixedText();
116111 }
 112+ if ($fld_sortkey)
 113+ $vals['sortkey'] = $row->cl_sortkey;
 114+ $data[] = $vals;
117115 } else {
118116 $resultPageSet->processDbRow($row);
119117 }
Index: trunk/phase3/includes/api/ApiQueryUserContributions.php
——@@ -172,10 +172,6 @@
173173 */
174174 private function extractRowInfo($row) {
175175
176 - $title = Title :: makeTitle($row->page_namespace, $row->page_title);
177 - if (!$title->userCanRead())
178 - return false;
179 -
180176 $vals = array();
181177
182178 if ($this->fld_ids) {
——@@ -185,7 +181,8 @@
186182 }
187183
188184 if ($this->fld_title)
189 - ApiQueryBase :: addTitleInfo($vals, $title);
 185+ ApiQueryBase :: addTitleInfo($vals,
 186+ Title :: makeTitle($row->page_namespace, $row->page_title));
190187
191188 if ($this->fld_timestamp)
192189 $vals['timestamp'] = wfTimestamp(TS_ISO_8601, $row->rev_timestamp);
Index: trunk/phase3/includes/api/ApiQueryImages.php
——@@ -77,12 +77,8 @@
7878 $lastId = $row->il_from;
7979 }
8080
81 - $title = Title :: makeTitle(NS_IMAGE, $row->il_to);
82 - // do not check userCanRead() -- page content is already accessible,
83 - // and images are listed there.
84 -
8581 $vals = array();
86 - ApiQueryBase :: addTitleInfo($vals, $title, true);
 82+ ApiQueryBase :: addTitleInfo($vals, Title :: makeTitle(NS_IMAGE, $row->il_to));
8783 $data[] = $vals;
8884 }
8985
Index: trunk/phase3/includes/api/ApiQueryLogEvents.php
——@@ -108,13 +108,10 @@
109109 }
110110
111111 private function extractRowInfo($row) {
112 - $title = Title :: makeTitle($row->log_namespace, $row->log_title);
113 - if (!$title->userCanRead())
114 - return false;
115 -
116112 $vals = array();
117113
118114 $vals['pageid'] = intval($row->page_id);
 115+ $title = Title :: makeTitle($row->log_namespace, $row->log_title);
119116 ApiQueryBase :: addTitleInfo($vals, $title);
120117 $vals['type'] = $row->log_type;
121118 $vals['action'] = $row->log_action;
——@@ -126,7 +123,7 @@
127124 if (isset ($params[0])) {
128125 $title = Title :: newFromText($params[0]);
129126 if ($title) {
130 - ApiQueryBase :: addTitleInfo($vals, $title, false, "new_");
 127+ ApiQueryBase :: addTitleInfo($vals, $title, "new_");
131128 $params = null;
132129 }
133130 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r24096Merged revisions 23910-24094 via svnmerge from...david22:38, 14 July 2007

Status & tagging log