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

HTTP code 304 being treated as an error #77

Closed
ydahhrk opened this issue Mar 10, 2022 · 1 comment
Closed

HTTP code 304 being treated as an error #77

ydahhrk opened this issue Mar 10, 2022 · 1 comment
Milestone

Comments

@ydahhrk
Copy link
Member

ydahhrk commented Mar 10, 2022

HTTP 304 is treated as an error because Fort assumes a successful download implies there's a file to rename.

As a result, the If-Modified-Since header is pure liability.

Assuming the code review is correct, this is causing lots of dropped files from the cache after the first validation iteration in server mode.

Branch rrdp-refactor has a WIP of this bug.

ydahhrk added a commit that referenced this issue Sep 9, 2023
It's a bit smarter now. Addresses a bunch of issues at once, though it
still needs several tweaks and testing:

- #78: Provide a dedicated namespace for each RRDP notification, to
  prevent malicious RPPs from overriding files from other RPPs.
- #79: RRDP session and serial are no longer cached in RAM; they're
  extracted from cached notification files as they are needed.
  This prevents all RRDP from being considered outdated during startup.
- #80: rsync-strategy has been removed.
- #81: The cache now retains RRDP files.

The refactor has been more intrusive than intended. I've been retouching
the core loop and rrdp/https code, which has yielded the following
further disinfections:

- #77: Refactor the HTTP code so 304 is handled as success, despite no
  file modifications having been made.
- It seems the old code was refusing to download RPPs via RRDP when said
  RPP wasn't also (unrelatedly) served via rsync. This seemed to stem
  from an old RFC misunderstanding from the previous developer.
- I've deprecated `rsync.priority` and `rrdp.priority`, mostly just to
  simplify the code. I haven't seen anyone using these config fields,
  and I think SIAs and/or randomness should be the ones to decide which
  protocol is preferred for a given RPP, not Fort's admin.
- However, I have also decided to deprecate `shuffle_tal_uris`, because
  I also suspect it's completely unused, and would like to hear some
  complaints otherwise.
- Deprecated `rsync.arguments-flat`, because non-recursive rsyncs are
  not needed anymore.
- Since RRDP files are no longer deleted immediately after use, the
  `DEBUG_RRDP` compilation has lost its purpose, so I deleted it.
- The code was using `HASH_ADD_STR` on strings contained outside of the
  node structure. This is illegal according to uthash's documentation,
  and might have induced some crashes in the past.
@ydahhrk ydahhrk added this to the v1.6.0 milestone Oct 6, 2023
@ydahhrk
Copy link
Member Author

ydahhrk commented Dec 1, 2023

Fort 1.6.0 has been released; closing.

@ydahhrk ydahhrk closed this as completed Dec 1, 2023
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

No branches or pull requests

1 participant