Page MenuHomePhabricator

[Bug] do not allow adding non-existent images in statements
Closed, ResolvedPublic

Event Timeline

Lydia_Pintscher raised the priority of this task from to Medium.
Lydia_Pintscher updated the task description. (Show Details)
JanZerebecki renamed this task from do not allow adding non-existent images in statements to [Story] do not allow adding non-existent images in statements.Sep 25 2015, 8:18 PM
JanZerebecki added a project: Story.
JanZerebecki set Security to None.
JanZerebecki added subscribers: Steinsplitter, Nikki.

The behaviour here has changed. Until recently I would get an error if I didn't select a valid file (although it seems from this older ticket that there was a way to get around the check). Now there appears to be no check at all and I'm able to save whatever I want as the value, e.g.

While I did used to see people edit image statements to non-existent files from time to time, since a few days ago I've seen a lot more (i.e. multiple times a day) and it's usually people adding statements with external URLs (like I did in the link above), which is not something I used to see at all.

I think there was a change to handle more validation in PHP only instead of duplicating it in JS, perhaps the change was because some additional validation happened in JS and thus was removed without noticing that that would be UI behaviour change.

@thiemowmde @Jonas Any idea if that is the case?

I can confirm that it is possible to link to random non-existing image names, see for example

The situation seems to be as follows:

  • The backend applies validation to all API input (from the UI and from bots). For CommonsMedia, this does not (and never did) check whether the image exists, it just checks whether the input is a syntactically valid image name.
  • The UI provides suggestions while the user is typing input. It does however allow the user to enter something that does not match any suggestion.

I suspect that in the past, the UI applied an extra validation step on the user input, which got dropped when we consolidated all validation in the backend.

Possible solutions:

  • Make backend validation check image existence. This would make it impossible to link non-existing images. It may (depending on how things are implamented) also make it impossibel to edit statements that link images that got deleted - it would not be possible to change the rank, or qualifiers, and the resulting error message may be confusing (the error has nothing to do with the edit the change the user was trying to make).
  • Make the input widget in the UI allow only input that matches a suggestion. I suspect this was the case in the past (or an extra validation step was applied). This would not prevent non-existing images to referenced via the API directly, so bots could still create "bad" image links.

The first solution sounds better to me. I'm not sure why we would want to keep links to deleted images (I seem to remember the statements being deleted when an image gets deleted on Commons, but maybe I'm confusing it with something else) or why we would want to edit such a statement without fixing the image. If it would generate a confusing error message, it seems like the error message should be improved. :)

The second sounds like it would revert the behaviour back to how it used to work, which would be better than the current behaviour, but it wouldn't solve the original problem, since changes like the one in the description would still be possible.

Lydia_Pintscher renamed this task from [Story] do not allow adding non-existent images in statements to [Bug] do not allow adding non-existent images in statements.Sep 30 2015, 12:26 PM
Lydia_Pintscher raised the priority of this task from Medium to High.
Lydia_Pintscher edited projects, added Regression; removed Story.

Change 260715 had a related patch set uploaded (by Hoo man):
Implement validation for CommonsMedia

Change 260715 merged by jenkins-bot:
Validate CommonsMedia file existence

hoo claimed this task.
hoo removed a project: Patch-For-Review.