Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simultaneous edits to an issue/PR lead to data loss #22907

Closed
drsybren opened this issue Feb 14, 2023 · 6 comments · Fixed by #31053
Closed

Simultaneous edits to an issue/PR lead to data loss #22907

drsybren opened this issue Feb 14, 2023 · 6 comments · Fixed by #31053
Labels

Comments

@drsybren
Copy link
Contributor

Description

TL;DR: When two people edit an issue/PR description simultaneously, the last one to save "wins".

This scenario is not uncommon:

  • User files a bug report.
  • Developer sees the report, and wants to edit it to clarify some wording or clean up some markdown.
  • User sees a typo, and edits the report.
  • Developer knows the system, so is faster, and saves their edits first.
  • User saves their typo fix
  • Now all the edits done by the developer are reverted by the edit from the user.

This problem is quite serious, because:

  1. There is nothing at all that warns about overwriting someone else's changes.
  2. There are no historical comments "user X changed the description".
  3. There is no way to see the history of description changes.

So not only is there data loss, it remains undetected until someone revisits the page and actually notices their work is gone.

A possible first step could be to use an etag to detect simultaneous edits. Then at least a user can be warned that they'll overwrite someone else's changes.

It would be better to have some way to merge those changes, but that's a whole different issue to solve. I think the most important is to prevent these losses; how to solve such collisions gracefully can be dealt with later.

Gitea Version

00b18ab

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Self-built from Git.

Database

PostgreSQL

@wxiaoguang
Copy link
Contributor

The only solution I can see is to completely refactor the issue edit feature to use AJAX, then various problems could be resolved and it's more friendly to users if something wrong happens.

@drsybren
Copy link
Contributor Author

drsybren commented Feb 14, 2023

That would be a good idea (to use the API for more aspects of the web interface), but it is far from the only solution.

A simple solution would be to:

  • Define a function H() that can compute some etag for the to-be-edited data. In this case, since the description can be edited all by itself, a SHA256 hash of that description would be good enough.
  • Include this hash into the form, and when POSTing the form, send it along in a If-Match: header.
  • Before saving to the database, call H() and compare it to the value of the If-Match: header. If they do not match, refuse to overwrite the data and re-render the edit template with the submitted data (so their edits are not lost) but with the new etag in there.
  • The user can open the issue in another tab (Gitea can provide a link with target="_blank" for convenience) to see what the other person did, and manually merge the differences.
  • A second "Save" can then overwrite; JS can be added to add yet another confirmation that older data should be overwritten.

This doesn't require a big refactor of the edit feature, and does provide an initial step to protect against this hidden data loss.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Feb 14, 2023

  • re-render the edit template with the submitted data

That's the hardest (nearly impossible at the moment) part without AJAX (in my opinion). That's why I proposed AJAX 😂

@drsybren
Copy link
Contributor Author

drsybren commented Feb 14, 2023

Hmmm then it looks like an inconvenient decision was made when designing the form -- either do it in JS all the way, or do it the traditional HTTP/HTML way. Sitting somewhere in the middle of those two is troublesome indeed.

In any case, I think this is a high priority issue, as the safety of data in issue descriptions is kind of central to Gitea. If this can't be solved quickly, maybe some pre-flight request made with JS before the POST happens can be added on short notice? A quick OPTIONS request that can be handled to check for collissions is not perfect, but at least it narrows the window of undetected losses to a few seconds (the time between the OPTIONS and the POST request).

@lunny
Copy link
Member

lunny commented Feb 14, 2023

We can have a version column for the issue table, so only one could update that column, and another will fail, but the form content will be kept on his web browser.

@drsybren
Copy link
Contributor Author

Thanks for addressing this :)

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants