nextcloud / contacts

📇 Contacts app for Nextcloud
https://apps.nextcloud.com/apps/contacts
GNU Affero General Public License v3.0
569 stars 173 forks source link

ETag header is stripped by CloudFlare, causing "The contact you were trying to edit has changed" to show after every edit #1157

Closed pirate closed 5 years ago

pirate commented 5 years ago

Appears to be superficially similar to https://github.com/nextcloud/contacts/issues/842.

The difference is I'm running on the latest version, but behind CloudFlare.

Anytime I stop typing for ~200ms, then try to continue, it loses all the edits and resets the contact to the previous version. This makes the web UI unusable as I lose dozens of contact field changes every time I try to update a contact. None of the edits I was making after the popup appeared were conflicting changes, they were strictly append-only and should've been easily resolvable.

(Note: I'm not sure that it's exactly 200ms before the next request is triggered, but it's something close to that and I'll just use that number for easier describing)

(scroll to the comment below this for the full Nextcloud issue template)

Screen Shot 2019-07-02 at 7 15 30 PM

Here's the console error generated when subsequent edits are sent to the server after the popup appears.

Screen Shot 2019-07-02 at 7 19 56 PM

(side-note: it would be nice to unlock #842 or mention this issue on it so people who find the old one via search can subscribe to this issue for updates)

pirate commented 5 years ago

I don't think most of the issue template is relevant, since it seems like a simple syncing logic problem and it matches the description in #842 , but here it is anyway:


Steps to reproduce

  1. Click on an existing contact via Web UI
  2. start typing in any field
  3. pause for ~200ms
  4. continue typing
  5. Popup is displayed, warning that "The contact you were trying to edit has changed"
  6. click refresh
  7. all edits after the popup was displayed are reverted and the changes lost

Expected behavior

The edits should persist when they are append-only and do not conflict with the previous version. Pausing for 200ms while typing should not have an effect on the UX.

Actual behavior

Edits are lost.

Server configuration detail

Operating system: Linux 5.0.0-17-generic #18-Ubuntu SMP Tue Jun 4 15:34:08 UTC 2019 x86_64

Webserver: Apache/2.4.25 (Debian) (apache2handler)

Database: pgsql PostgreSQL 9.6.14 on x86_64-pc-linux-musl, compiled by gcc (Alpine 8.3.0) 8.3.0, 64-bit

PHP version:

7.3.6 Modules loaded: Core, date, libxml, openssl, pcre, sqlite3, zlib, ctype, curl, dom, fileinfo, filter, ftp, hash, iconv, json, mbstring, SPL, PDO, bz2, posix, Reflection, session, SimpleXML, pdo_sqlite, standard, tokenizer, xml, xmlreader, xmlwriter, mysqlnd, apache2handler, apcu, Phar, exif, gd, gmp, imagick, imap, intl, ldap, memcached, pcntl, pdo_mysql, pdo_pgsql, redis, smbclient, sodium, zip, libsmbclient, Zend OPcache

Nextcloud version: 16.0.1 - 16.0.1.1

Updated from an older Nextcloud/ownCloud or fresh install: Fresh

Where did you install Nextcloud from: docker

Signing status Valid.
List of activated apps ``` Enabled: - activity: 2.9.1 - admin_audit: 1.6.0 - apporder: 0.7.1 - bruteforcesettings: 1.3.0 - calendar: 1.7.0 - checksum: 0.4.3 - cloud_federation_api: 0.2.0 - comments: 1.6.0 - contacts: 3.1.3 - dav: 1.9.2 - event_update_notification: 0.3.4 - external: 3.3.0 - federatedfilesharing: 1.6.0 - federation: 1.6.0 - files: 1.11.0 - files_accesscontrol: 1.6.0 - files_external: 1.7.0 - files_fulltextsearch: 1.3.2 - files_fulltextsearch_tesseract: 1.3.0 - files_markdown: 2.0.6 - files_pdfviewer: 1.5.0 - files_readmemd: 1.1.0 - files_rightclick: 0.13.0 - files_sharing: 1.8.0 - files_snapshots: 0.2.0 - files_texteditor: 2.8.0 - files_trashbin: 1.6.0 - files_versions: 1.9.0 - files_videoplayer: 1.5.0 - firstrunwizard: 2.5.0 - forms: 1.0.3 - fulltextsearch: 1.3.4 - gallery: 18.3.0 - groupfolders: 4.0.3 - guests: 1.1.0 - impersonate: 1.3.0 - issuetemplate: 0.5.0 - logreader: 2.1.0 - lookup_server_connector: 1.4.0 - metadata: 0.9.0 - nextcloud_announcements: 1.5.0 - notes: 3.0.0 - notifications: 2.4.1 - oauth2: 1.4.2 - passman: 2.3.1 - password_policy: 1.6.0 - polls: 0.10.2 - privacy: 1.0.0 - provisioning_api: 1.6.0 - recommendations: 0.4.0 - registration: 0.4.6 - richdocuments: 3.3.12 - serverinfo: 1.6.0 - sharebymail: 1.6.0 - smb_test: 0.2.2 - socialsharing_email: 1.0.5 - spreed: 6.0.2 - support: 1.0.0 - survey_client: 1.4.0 - suspicious_login: 1.0.0 - systemtags: 1.6.0 - theming: 1.7.0 - theming_customcss: 1.3.0 - twofactor_backupcodes: 1.5.0 - updatenotification: 1.6.0 - video_converter: 0.1.0 - viewer: 1.0.0 - workflowengine: 1.6.0 Disabled: - accessibility - admin_notifications - announcementcenter - dashboard - encryption - files_inotify - fulltextsearch_elasticsearch - githubmergetracker - news - nextant - socialsharing_twitter - spreedme - tasks - user_ldap ```
Configuration (config/config.php) ``` { "memcache.local": "\\OC\\Memcache\\APCu", "apps_paths": [ { "path": "\/var\/www\/html\/apps", "url": "\/apps", "writable": false }, { "path": "\/var\/www\/html\/custom_apps", "url": "\/custom_apps", "writable": true } ], "instanceid": "***REMOVED SENSITIVE VALUE***", "passwordsalt": "***REMOVED SENSITIVE VALUE***", "secret": "***REMOVED SENSITIVE VALUE***", "trusted_domains": [ "cloud.monadical.com" ], "datadirectory": "***REMOVED SENSITIVE VALUE***", "overwrite.cli.url": "https:\/\/cloud.monadical.com", "overwritehost": "cloud.monadical.com", "overwriteprotocol": "https", "dbtype": "pgsql", "version": "16.0.1.1", "dbname": "***REMOVED SENSITIVE VALUE***", "dbhost": "***REMOVED SENSITIVE VALUE***", "dbport": "5432", "dbtableprefix": "oc_", "dbuser": "***REMOVED SENSITIVE VALUE***", "dbpassword": "***REMOVED SENSITIVE VALUE***", "installed": true, "mail_smtpmode": "smtp", "mail_smtpauthtype": "LOGIN", "mail_smtpsecure": "tls", "mail_from_address": "***REMOVED SENSITIVE VALUE***", "mail_domain": "***REMOVED SENSITIVE VALUE***", "mail_smtpauth": 1, "mail_smtphost": "***REMOVED SENSITIVE VALUE***", "mail_smtpport": "587", "mail_smtpname": "***REMOVED SENSITIVE VALUE***", "mail_smtppassword": "***REMOVED SENSITIVE VALUE***", "Theme": "DarkCloud", "maintenance": false, "theme": "", "loglevel": 2, "app_install_overwrite": [ "calendar" ] } ```

Are you using external storage, if yes which one: NA

Are you using encryption: false

Are you using an external user-backend, if yes which one: None (using web UI)

Client configuration

Note: client config is not relevant, this is reproducible on multiple different client browsers and OSs.

Browser: Google Chrome v76.0.3809.46 (Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.46 Safari/537.36)

Operating system: macOS 10.15 (Not relevant, repr

Logs

Browser log ```javascript Failed to load resource: the server responded with a status of 412 () contacts.js:354 Error at XMLHttpRequest.s.onreadystatechange (request.js:353) contacts.js:360 This contact is outdated, the server refused it t (anonymous) @ contacts.js:360 ```

(see screenshots above)

skjnldsv commented 5 years ago

Hello! :) Thanks for reporting! I will need a screenshot of your network requests. Before you do anything, still on the development tools, there is a tab called network. Click on it and then the xhr filter. Do your action and screenshot the network log like this: capture d ecran_2018-09-27_21-49-46

pirate commented 5 years ago

I looks like the culprit might be the frontend sending an if-match: null header in the request.

Screen Shot 2019-07-09 at 1 39 00 PM Screen Shot 2019-07-09 at 1 39 12 PM

Edit: removed request details to put them below together for easier reference (see next comment below).

I can provide more detailed screenshots / copy paste of the request/repsonse data if needed.

skjnldsv commented 5 years ago

So, it always happens on the second request ? Could you get me the response headers from the first request please?

pirate commented 5 years ago

It's not always the second request, it's whatever request happens after a ~200ms+ delay in typing, I just triggered it immediately when I was getting the screenshots because that was quickest. Here's an example of making two successful edits by typing quickly, then pausing and making a third edit where it fails:

Screen Shot 2019-07-12 at 2 39 58 AM

Do you think it might be related to CloudFlare potentially messing with the etag / cache headers? If so maybe it's better if the web UI also adds its own headers instead of relying on the HTTP standard ones that middle-boxes often mess with? I know I'm definitely not the only one running NextCloud behind CloudFlare though, so I'd be surprised if this is being caused by CF and no one else is reporting this issue. As mentioned above, it looks like the smoking gun is more likely the frontend sending an if-match: null header in the request.


For easier debugging, I made two edits and captured the headers and payloads of both in sequence. The first edit was typed quickly and succeeded, the second edit was typed a 200ms delay after the first one, and it fails.

First Request (succeeds)

Here's the info for the first edit request after typing "first edit" in the Name section's Prefix field without pausing during typing. All other edit requests that are made without pauses in typing succeed similarly.

General: ``` Request URL: https://cloud.monadical.com/remote.php/dav/addressbooks/users/nick/Employees/7151a67c-29f3-45c1-a07e-6d481d967740.vcf Request Method: PUT Status Code: 204 Remote Address: 104.18.59.73:443 Referrer Policy: no-referrer ```
Request headers: ``` :authority: cloud.monadical.com :method: PUT :path: /remote.php/dav/addressbooks/users/nick/Employees/7151a67c-29f3-45c1-a07e-6d481d967740.vcf :scheme: https accept: */* accept-encoding: gzip, deflate, br accept-language: en-US,en;q=0.9 cache-control: no-cache content-length: 668455 content-type: application/xml; charset=UTF-8 cookie: depth: 0 dnt: 1 if-match: "36f476afad70ee8d8afce0959ec7dd1b" origin: https://cloud.monadical.com pragma: no-cache requesttoken: sec-fetch-mode: cors sec-fetch-site: same-origin user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.46 Safari/537.36 x-requested-with: XMLHttpRequest ```
Response headers: ``` cache-control: no-store, no-cache, must-revalidate cf-ray: 4f50e92d7e6ec1ed-IAD content-security-policy: default-src 'none'; date: Fri, 12 Jul 2019 06:20:22 GMT expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct" expires: Thu, 19 Nov 1981 08:52:00 GMT oc-etag: "b2c3ca15d7280f9836e4d0c09b75711a" pragma: no-cache referrer-policy: no-referrer server: cloudflare status: 204 strict-transport-security: max-age=15552000; preload x-content-type-options: nosniff x-download-options: noopen x-frame-options: SAMEORIGIN x-permitted-cross-domain-policies: none x-powered-by: PHP/7.3.6 x-robots-tag: none x-xss-protection: 1; mode=block ```
Payload: ``` BEGIN:VCARD VERSION:3.0 PRODID:-//Apple Inc.//Mac OS X 10.14.4//EN N:Sweeting;Nicholas;Dunham;first edit; FN:Nicholas Sweeting ... ```
Response Data: ``` None (204 status) ```

Second request (fails)

This is the second request made after waiting ~200ms then continuing to type "second edit" after "first edit" into the Name Prefix field. All other requests made after a delay in typing result in similar failures.

General: ``` Request URL: https://cloud.monadical.com/remote.php/dav/addressbooks/users/nick/Employees/7151a67c-29f3-45c1-a07e-6d481d967740.vcf Request Method: PUT Status Code: 412 Remote Address: 104.18.59.73:443 Referrer Policy: no-referrer ```
Request headers: ``` :authority: cloud.monadical.com :method: PUT :path: /remote.php/dav/addressbooks/users/nick/Employees/7151a67c-29f3-45c1-a07e-6d481d967740.vcf :scheme: https accept: */* accept-encoding: gzip, deflate, br accept-language: en-US,en;q=0.9 cache-control: no-cache content-length: 668466 content-type: application/xml; charset=UTF-8 cookie: depth: 0 dnt: 1 if-match: null origin: https://cloud.monadical.com pragma: no-cache requesttoken: sec-fetch-mode: cors sec-fetch-site: same-origin user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.46 Safari/537.36 x-requested-with: XMLHttpRequest ```
Response headers: ``` cache-control: no-store, no-cache, must-revalidate cf-ray: 4f50e93e4d92c1ed-IAD content-length: 312 content-security-policy: default-src 'none'; content-type: application/xml; charset=utf-8 date: Fri, 12 Jul 2019 06:20:25 GMT etag: "b2c3ca15d7280f9836e4d0c09b75711a" expect-ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct" expires: Thu, 19 Nov 1981 08:52:00 GMT pragma: no-cache referrer-policy: no-referrer server: cloudflare status: 412 strict-transport-security: max-age=15552000; preload x-content-type-options: nosniff x-download-options: noopen x-frame-options: SAMEORIGIN x-permitted-cross-domain-policies: none x-powered-by: PHP/7.3.6 x-robots-tag: none x-xss-protection: 1; mode=block ```
Payload: ``` BEGIN:VCARD VERSION:3.0 PRODID:-//Apple Inc.//Mac OS X 10.14.4//EN N:Sweeting;Nicholas;Dunham;first edit second edit; FN:Nicholas Sweeting ... ```
Response Data: ``` Sabre\DAV\Exception\PreconditionFailed An If-Match header was specified, but none of the specified the ETags matched. If-Match ```

(Minor unrelated thing: there's a typo in the error message)

- An If-Match header was specified, but none of the specified the ETags matched.
+ An If-Match header was specified, but none of the specified ETags matched.
skjnldsv commented 5 years ago

@georgehrke any reason the etag is not return but only oc-etag on the first PUT?

georgehrke commented 5 years ago

@skjnldsv Is that always the case or just sometimes?

georgehrke commented 5 years ago

I think the problem here is not that the first request does not return an ETAG. That's neglectable. The problem is that c-dav sends an If-Match: null, which will obviously always error.

Let me look into that.

skjnldsv commented 5 years ago

@georgehrke but if the if-match is null this is because an etag is not returned :)

georgehrke commented 5 years ago

@skjnldsv That's indeed what's causing the trouble here, but it is allowed as per RFC 6352.

@pirate I see some cloud flare references in the headers. Maybe your cloud flare config causes it to filter ETAG headers for certain requests?

pirate commented 5 years ago

(see above)

Do you think it might be related to CloudFlare potentially messing with the etag / cache headers? If so maybe it's better if the web UI also adds its own headers instead of relying on the HTTP standard ones that middle-boxes often mess with? I know I'm definitely not the only one running NextCloud behind CloudFlare though, so I'd be surprised if this is being caused by CF and no one else is reporting this issue.

I suspected that earlier, but even if that is the cause, it would be nice if it didn't break behind CloudFlare, I don't think I'm willing to remove that layer as it's doing other things for us (and I know we're certainly not the only ones running NextCloud behind CF). Have any workaround ideas?

Our CloudFlare settings are also already as close as I am able to get them to "don't mess with caching headers". CloudFlare also claims to support passing ETag headers.

There may be a relevant section here though:

Weak ETag headers are fully supported on Cloudflare by default, and are available for all plan levels. ...

Currently strong ETag headers are only supported for Cloudflare Enterprise customers, and need to be manually activated on URLs using a Page Rule. If a Page Rule is not set, then any strong ETag headers will be converted into weak ETag headers.

Using an ETag without quotes (Etag: blah for example) causes the gzip module to remove a strong ETag instead of weakening it. Use quotes (etag: "blah" for example) for the strong ETag to weaken into etag: W/"blah".

Maybe adding ETag: W/"..." or adding quotes might fix it? That way it'll get downgraded to a weak ETag instead of being removed entirely?

georgehrke commented 5 years ago

@skjnldsv et voilĂ  https://github.com/nextcloud/cdav-library/pull/164

pirate commented 5 years ago

Wow, crazy fast, thank you @georgehrke! Just to clarify, will it still be able to handle append-only style changes without requiring a refresh if there's no ETag header to check? Might it be worth trying to add quotes as well so the ETag header can make it through CloudFlare intact?

skjnldsv commented 5 years ago

@georgehrke but then it doesn't solve the etag dot being there, right?

georgehrke commented 5 years ago

Just to clarify, will it still be able to handle append-only style changes without requiring a refresh if there's no ETag header to check?

That question I will leave up to our Contacts App dev @skjnldsv

Might it be worth trying to add quotes as well so the ETag header can make it through CloudFlare intact?

We have to send the exact same ETag that we get from the server, otherwise the request will always fail. If we want to do that, that's something we have to fix in the CardDAV server itself in https://github.com/nextcloud/server. First of all, changing the structure of an ETag is something I'm only willing to do after careful testing with various clients and definitely not in a minor / patch update. Furthermore we should also be careful with that, because all clients that don't use RFC 6578 will think that all events / vcards changed and request all of them at one time, causing a huge load.

@georgehrke but then it doesn't solve the etag not being there, right?

No, so changes may be overridden, yes. If you want me to, I can also add a second patch that takes oc-etag if ETAG is not set.

skjnldsv commented 5 years ago

Just to clarify, will it still be able to handle append-only style changes without requiring a refresh if there's no ETag header to check?

Well, no etag header will only allow you to do one push per contact before refresh. There is no way around it :/

georgehrke commented 5 years ago

Well, no etag header will only allow you to do one push per contact before refresh. There is no way around it :/

It should work anyway, it will just never send an If-Match header, meaning that conflicts can't be resolved/detected, but rather other changes that took place in the meantime will simply be overridden.

skjnldsv commented 5 years ago

Oh, ok!

pirate commented 5 years ago

For us at least, I think we're perfectly ok with last-write-wins behavior when ETags aren't present, feel free to close this issue whenever / once that PR is merged :)

skjnldsv commented 5 years ago

Implemented in https://github.com/nextcloud/cdav-library/pull/164 Will be available in next contacts release :)

cedricbonhomme commented 2 years ago

Hi @pirate @georgehrke @skjnldsv !

I have the same problem with last verion of Nextcloud (24.0.5) and Contacts.

Editing a contact is nearly impossible. I have 2 seconds to leave an input after starting to type. Else I have the sane problem than here

When I start to edit a contact, the first request contains a oc-etag and an If-Match. The second request contains a etag and an If-Match. So no oc-etag The values of If-Match are not null.

I was so bored* with this issue that I solved the problem by changing this line : https://github.com/sabre-io/dav/blob/master/lib/DAV/Server.php#L1311 to true ;-)

Now the issue is gone, since we don't go in this section of code. Contacts works quite well, seems no indirect impacts.

As a side note, in this case it's a very new Nextcloud instance dedicated to a Music School. I am using Nextcloud since +5 years on some personal servers and even at work. My oldest (and personal) instance always worked pretty well for editing contacts. And now I realize that the issue is as well on this instance ! I could solve the problem as well on my own instance with this hack... but not for now.

* I have to enter more than 70 contacts (with lot of information for each contact) for this music school. And manually since all the information is on paper. So I can say it's not really possible to work with this issue.

megavolts commented 2 years ago

Hi @skjnldsv, @georgehrke, @pirate.

I currently experienced the same problem as @cedricbonhomme with Nextcloud 24.0.6 and Contacts 4.2.2.

I applied the same fix, and solved temporary the problem.

nextcloud is install in a docker and accessed through cloudflared