#1333 Comment is lost
Closed: Fixed Opened by cqi.

Reproduce steps

  • open an issue, and input text in comment textarea
  • press Preview button, the preview content is shown
  • stay for a while, maybe several minutes (I forgot how long I left the page untouched)
  • Press Update Issue button

The comment is not shown at last. Reload page, still not shown. Re-enter that issue page, still not there. It's lost.

I'm using Firefox 48.0.1.


I would be curious if this is still happening these days

It just happened to me. I didn't click preview though, or wait a particularly long time.

I've only seen the issue just a handful of times, but it is particularly frustrating when it happens.

Yes it is happening alright.

Edit similarly edits are not rendered.

Test comment from incognito firefox.

Test comment from chromium.

Another comment from chromium with disabled cache in debug console.

from discussion on #fedora-devel:

[...] i'd recommend you leave a note that returning an explicit Expires header might resolve your issue

Anyway I am pretty sure this is a pagure regression - this problem didn't occur earlier.

Well, this ticket is already a month old so...

Btw, what is the difference between a regression and a bug?

And the behavior we're currently seeing has nothing to do with the original issue reported in this ticket.

Here the comment aren't been displayed but they are sent and reloading the page fixes the display. So the current issue is basically the issue we've been having with the SSE server for a while, cf #462. All it takes to fix it (temporarily) is to restart that service (which I just did)

The present ticket is about comment disappearing without actually being sent and thus without re-appearing on reload.

Well indeed you can't see the comments I tried to post in pagure but were not. :-)
I am aware of the issue and that is why I came here.

The rendering issue is forgivable but not losing posts silently. :)

Anyway thanks, I appreciate this is a tricky problem from the discussion in other ticket.

Would it be possible/make sense to restart the server at regular intervals say as a workaround in the meantime to hide/not expose the problem from/to our users?

It might solve #462 but it won't solve the issue reported here

I have not really been able to reproduce this, but I decided to take a look at the code again this morning and I know what must be happening.

The form being submitted is failing validation here. I don't know why, maybe because the session expired or something like that, I'll have to learn more about that. However, because it fails validation, the next line executed is here, where the server returns a nice HTTP 200.

It seems like instead, the server should return an error of some kind (is HTTP 400 appropriate here?) and the JavaScript should check the response and, at the least, not clear the field.

I do agree that there seems to be a flaw in the logic but I was able to reproduce the issue a couple of time (more by accident than in purpose) and I've seen that the ajax call triggers a 302 response which reloads the page w/o either sending the form or conserving its content, which leads to the behavior we're seeing.
But I do not know how/where this 302 is triggered, I need to find a way to replicate this :s

Muhahah, I found it :)

So we thought we had fixed it in: https://infrastructure.fedoraproject.org/cgit/ansible.git/commit/?id=029f5225 where we basically tell flask-wtf to not put a time constraint on the CSRF token provided.

Thanks to @kevin I was able to reproduce the error locally and what I saw in the logs was:
- the CSRF token was provided
- the form returned an error: missing CSRF token

This kinda got me thinking, so I went to look at the sources of both flask-wtf and wtforms. In prod we are using flask-wtf 0.8 and wtforms 2.0.

And this is what I found:
- validate_csrf_token calls: super(Form, self).validate_csrf_token(field) where super is from wtforms.ext.csrf.session import SessionSecureForm (cf https://github.com/lepture/flask-wtf/blob/4567df1cf12c2e5fd578d373ee35dafc9f3a8f47/flask_wtf/form.py#L82-L85)
- in wtforms/ext/csrf/session.py the method validate_csrf_token does:

    if self.TIME_LIMIT:
        now_formatted = datetime.now().strftime(self.TIME_FORMAT)
        if now_formatted > expires:
            raise ValidationError(field.gettext('CSRF token expired'))

https://github.com/wtforms/wtforms/blob/c53f8a3d8b14536810cb71cab0730673d9f7d566/wtforms/ext/csrf/session.py#L68-L71

Conclusion: the configuration key WTF_CSRF_TIME_LIMIT is just ignored which means: the CSRF token have a 30 minutes life-time, thus this annoying bug.

We should be able to fix it though by simply overriding the TIME_LIMIT in the form.

Hopefully the hack in https://pagure.io/pagure/pull-request/1523 should fix this once and for all :)

I'm going to close this one for now and we can re-open if the problem still exists in the next release

@pingou changed the status to Closed

Thanks!

Metadata