View Issue Details

IDProjectCategoryView StatusLast Update
0003499HTML & PERLBug Report - Interfacepublic2023-04-23 00:54
Reporterinim Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status newResolutionopen 
PlatformFirefox (latest)OSLinuxOS VersionUbuntu 20.4.2
Summary0003499: Error creating CREQ for for entry with 18+ tag leaves DB inconsistent afterwards
DescriptionAn attempt to modify the "nudity" content indicator for an entry with adult tags is rejected. In the DB, the "nudity" field is blanked after this, overwriting the old value and not setting the requested one.
Steps To ReproduceReproduce:
1. Open https://anidb.net/anime/6083 (Kiss x Sis OVA) in Web Browser
2. Tags -> Add / Edit Tags -> Edit Tags (GUI) -> content indicators
3. Change value of "nudity" content rating from existing 2.x value to 1.5 and add comment

Problem:
The creation of a CREQ to admin is rejected with this error message:
"* urination - Error: tag is restricted to adult content only "

After this, the "nudity" content rating is blanked, i.e. there is no star. Existing stars are removed and not replaced by the new value. They are just deleted. This way one can hack the site and remove all nudity content ratings.

Request:
I won't test further to keep the DB consistent. But this looks like a control flow problem where an error condition
(a) shouldn't occur, why not change non-adult tags / content ratings on an anime with adult tags?
(b) there seem to be side effects with DB modification due to a bug in the error handling

Please change the code so (a) becomes possible and fix (b) to stop this denial of service problem for DB consistency.
TagsNo tags attached.

Activities

DerIdiot

2021-04-20 14:25

administrator   ~0004466

Literally no data is being changed there and how is that a denial of service. Neither of these things applies.
The tga is restricted and you got the appropirate response. Nothing to do here

inim

2021-04-28 14:02

reporter   ~0004467

The word "denial of service" probably was too alarmist, I can't know for sure.

There still is a problem at least in the display. After that error message, no stars were displayed for the changed category anymore in my GUI. The 1.5 stars we see now were manually entered by somebody after my change attempt. I still think the something, be it the actual DB, the display layer whatever, is messed up by triggering the error described.

Hinoe

2023-04-23 00:54

reporter   ~0004486

I know this is old, but I think I should throw some cents at it.

According to entry history, the nudity tag for anime 6083 has never had its weight changed since being added in the old category system all the way back in 2010. No changes were made -- and you would not be able to make any changes to that tag-entity relation without someone approving them anyway. I expect your GUI derped, so it would be at most a javascript issue with no DB impct. If confirmed, that would be an issue in and of itself, but that GUI is painful enough that I would argue we don't want to touch it unless we have a breaking issue; in my opinion, what you saw is not a big enough issue to warrant that.

That said, there *is* a problem here that we probably want to see fixed. The problem is that, when there is one invalid tag assignment in the entry, the checks for invalid tag assignments are blocking all GUI-based massedits (possibly the text-based ones too? I don't know), irrespective of whether any change is being made to those invalid assignments. Because it blocks a bunch of legitimate edits for a completely unrelated reason and surprises the user (who also loses all the work done on the edits), I think that is a highly counterproductive behavior. The checks should only block changes that *create* new invalid assignments -- if an *already existing* invalid assignment is there but all *changes* are valid, it should process the changes. I'd say it should probably throw a warning about the invalid assignment, but, by and large, if such an assignment is there, although it is an error state, there is even a chance it is an intentional one. In this case, well, I haven't watched that anime, but, knowing its reputation, I wouldn't be one bit surprised if urination was an actual thing in it. The tag was eventually deleted from the anime, possibly only because it's an invalid assignment, and maybe the right thing would have been to keep the assignment and edit the tag to allow it on non-porn; I don't know. On the other hand, if you are changing an already invalid assignment in a way that makes it stays invalid, it should do something like ask for confirmation or throw a warning in the creq for the handler.

So, when faced with an invalid assignment, the decision-making process should probably be something more or less like this:

A) If the assignment was already there and isn't being touched: process all changes, maybe throw a warning.

B) If the assignment was already there and is being touched in a way that makes it stay invalid (change weight, toggle local spoiler flag): maybe ask for confirmation, arguably process the change but throw an automatic system message in the creq for the handler to decide what to do. Maybe throw the system message in the creq always and ask for confirmation only if the change is being done by a mod or maintainer, and block owneredits (generate a normal creq instead). If the creq is a child of another creq, maybe also throw a warning in the parent creq concerning the relevant child(ren), or maybe do not make parent-child relations for those creqs at all (if this is a feasible option). Those warnings should not be added with the issuer's uid but with system uid, so that the user cannot edit/delete them.

C) If the assignment was not there, then yes, continue to block and scream at the user as is done today. We don't want *new* invalid assignments. That said, if the user is a maintainer a mod, maybe don't block, but scream and ask for confirmation; we should likely assume that we want those people to be *able* to override those blocks in the event this is somehow necessary (maintainers and mods do already have the power to do that by changing the tagreltb properties back and forth if they *really* want to make an invalid assignment, it's just an incredibly annoying process), but we probably don't want anyone overriding blocks by mistake.

I imagine that decision-making thing should apply for both massedits as well as individual edits, including when someone edits a creq. And obviously, if the assignment was already there and is being fixed (to remove/add weight, as appropriate, or delete the tag), it should process the change normally with no warnings.

Issue History

Date Modified Username Field Change
2021-04-12 08:39 inim New Issue
2021-04-12 10:14 inim Summary Can't create CREQ for approval by admin for entry with 18+ tags => Error creating CREQ for for entry with 18+ tag leaves DB inconsistent afterwards
2021-04-20 14:25 DerIdiot Note Added: 0004466
2021-04-28 14:02 inim Note Added: 0004467
2021-09-06 15:48 DerIdiot Project AniDB Website => HTML & PERL
2023-04-23 00:54 Hinoe Note Added: 0004486