Last modified: 2013-02-28 18:18:10 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T46080, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 44080 - PublishStashedFile.php logs limited uploader information
PublishStashedFile.php logs limited uploader information
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
UploadWizard (Other open bugs)
master
All All
: Normal critical (vote)
: ---
Assigned To: Aaron Schulz
:
: 44074 (view as bug list)
Depends on:
Blocks: 34834
  Show dependency treegraph
 
Reported: 2013-01-17 22:53 UTC by Marcin Cieślak
Modified: 2013-02-28 18:18 UTC (History)
19 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Marcin Cieślak 2013-01-17 22:53:18 UTC
When uploading using [[Special:UploadWizard]] the image upload gets logged into the CheckUser log as:


(Rejestr operacji) . . 22:19 . . Saper (dyskusja | edycje | zablokuj) Saper uploaded "File:Screenshot one.png" (User created page with UploadWizard)
        IP: 127.0.0.1   

instead of

(Rejestr operacji) . . 22:37 . . Saper (dyskusja | edycje | zablokuj) Saper uploaded "File:No sighted version en.png" (User created page with UploadWizard)
        IP: 2A01:198:200:15F:0:0:0:2   Mozilla/5.0 (X11; FreeBSD amd64) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.57 Safari/536.11

This is because after commit a31d0f8edd167e9ee301b694a8092fe0cbcfdabf (Change ID: Iae38b93d65182158561b92168af51a1e9f50374c) the actual upload is handled by the command-line utility that does not receive all WebRequest metadata.

Reverting this commit has restored this functionality in my tests (even though there are multiple API requests in the log and only the final one adds the entry to the "image" table).
Comment 1 Huji 2013-01-18 00:46:13 UTC
*** Bug 44074 has been marked as a duplicate of this bug. ***
Comment 2 Rob Lanphier 2013-01-18 01:28:53 UTC
I believe Aaron has a live hack to fix this, and will come up with a more permanent fix.
Comment 3 Risker 2013-01-18 02:59:09 UTC
This seems to be a recurrent issue with various extensions (WikiLove, Translate in the past year), so it might be worthwhile to see if there is a common issue in the code that was initially uploaded, or the method of resolution. Might also be worthwhile to add "verify that IP address pulls through properly on Checkuser results" to new extensions prior to commit.
Comment 4 Risker 2013-01-18 03:06:43 UTC
(In reply to comment #3)
> This seems to be a recurrent issue with various extensions (WikiLove,
> Translate
> in the past year), so it might be worthwhile to see if there is a common
> issue
> in the code that was initially uploaded, or the method of resolution. Might
> also be worthwhile to add "verify that IP address pulls through properly on
> Checkuser results" to new extensions prior to commit.

Note previous bug 34838 and bug 35136  and this is also apparently associated with LiquidThreads.
Comment 5 Aaron Schulz 2013-01-18 19:23:16 UTC
(In reply to comment #2)
> I believe Aaron has a live hack to fix this, and will come up with a more
> permanent fix.

I think disabling it in master too is what I will do.
Comment 6 Aaron Schulz 2013-01-18 19:27:19 UTC
(In reply to comment #5)
> (In reply to comment #2)
> > I believe Aaron has a live hack to fix this, and will come up with a more
> > permanent fix.
> 
> I think disabling it in master too is what I will do.

https://gerrit.wikimedia.org/r/#/c/44693/

Done.
Comment 7 MZMcBride 2013-01-23 05:48:09 UTC
(In reply to comment #3)
> This seems to be a recurrent issue with various extensions (WikiLove,
> Translate in the past year), so it might be worthwhile to see if there
> is a common issue in the code that was initially uploaded, or the
> method of resolution. Might also be worthwhile to add "verify that IP
> address pulls through properly on Checkuser results" to new extensions
> prior to commit.

This is usually accomplished in the form unit tests, I believe.
Comment 8 Risker 2013-01-23 06:33:20 UTC
(In reply to comment #7)
> (In reply to comment #3)
> > This seems to be a recurrent issue with various extensions (WikiLove,
> > Translate in the past year), so it might be worthwhile to see if there
> > is a common issue in the code that was initially uploaded, or the
> > method of resolution. Might also be worthwhile to add "verify that IP
> > address pulls through properly on Checkuser results" to new extensions
> > prior to commit.
> 
> This is usually accomplished in the form unit tests, I believe.

Well, one would have thought.  Unfortunately, it seems that whatever test is being used has allowed the same problem to slip through on at least three occasions that I came up with from the past year. That means the test isn't testing what it needs to test in order to ensure that we don't get this result from the code.  It's a system problem, not a people problem; I have no doubt that the usual suite of tests were being done. However, we need a test that will tell us whether or not a CU will see 127.0.0.1 results instead of the actual originating IP and UA, and it appears that particular test isn't incorporated into the pre-release QA check yet.  I have faith that a method for testing this, in order to ensure that this particular question is adequately answered, will be developed.
Comment 9 Marcin Cieślak 2013-01-23 09:05:20 UTC
I think the solution might be to throw an exception on every case where an attempt is made to change something (let's say push a revision) where no valid WebRequest object is provided, containing User-Agent for example. Or some workaround is attempted via FauxRequest or half-baked API call.
Comment 10 Rob Lanphier 2013-02-01 22:46:10 UTC
Lowering priority since this isn't in production anymore.
Comment 11 Aaron Schulz 2013-02-28 18:18:10 UTC
https://gerrit.wikimedia.org/r/#/c/48940/

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links