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

internal: non-strict RFC ETag format support #174

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

borisovdev
Copy link

For some caldav providers, it wasn't possible to unmarshal unquoted text for etag

Referenced to #165, #69

@emersion
Copy link
Owner

This is non-standard, so not sure I want to maintain this.

This also unnecessarily uses regular expressions, and falls apart when marshaling etags.

@borisovdev
Copy link
Author

borisovdev commented Oct 26, 2024

This is non-standard, so not sure I want to maintain this.

This also unnecessarily uses regular expressions, and falls apart when marshaling etags.

Thanks for the quick reply and your time!
Yes, I have already familiarized myself with other similar issues. The reality is that many providers do not strictly follow the RFC in this place. Should we send people to live in a fork just because the rules say so? Often we do, but it seems not in this case. Please give it some thought:)

Thanks for pointing out MarshalText, I should be more careful. I'm marking PR as draft and will finalize it as soon as I have free time.

@borisovdev borisovdev marked this pull request as draft October 26, 2024 12:49
@borisovdev borisovdev force-pushed the fix/etag-unmarshal-text branch from 355f16f to fb85cd0 Compare October 27, 2024 14:41
@borisovdev
Copy link
Author

I like to clarify details, to avoid misunderstandings) We are talking about https://datatracker.ietf.org/doc/html/rfc2616#section-3.11, right?

About the review:

This also unnecessarily uses regular expressions and falls apart when marshaling etags.

  1. removed regexp from implementation
  2. I thought of possible problems with marshaling in my head, but didn't find them. In the extended implementation of UnmarshalText an unquoted string is returned anyway. I added new tests and modified the old one

This is non-standard, so not sure I want to maintain this.

the fate of this item is in your hands:)

Rebased and updated the branch

@borisovdev borisovdev marked this pull request as ready for review October 27, 2024 14:53
@Miha-ha
Copy link

Miha-ha commented Nov 25, 2024

I also encountered this problem when using DavMail. The edits are very minimal and I don't want to make a fork for this. I am sure that with these changes it will only get better!

For some caldav providers, it wasn't possible to unmarshal unquoted text for etag.
https://datatracker.ietf.org/doc/html/rfc2616#section-3.11 points to wrap text with quotes, but in reality it's sometimes ignored.

internal: test for ETag UnmarshalText, ETag String, ETag unmarshal and marshal
@borisovdev borisovdev force-pushed the fix/etag-unmarshal-text branch from fb85cd0 to 9d2522d Compare December 22, 2024 10:24
@borisovdev borisovdev changed the title internal: fixed ETag UnmarshalText problem for unquoted text internal: non-strict RFC ETag format support Dec 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants