Skip to content

feat(gateway): introduce typed errors for blocked content #855

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

Closed
wants to merge 2 commits into from

Conversation

PsychoPunkSage
Copy link
Contributor

Previously, blocked content detection relied on string matching which is fragile and implementation-specific. This PR:

  • Adds ErrorContentBlocked type to properly handle blocked content errors
  • Allows configurable HTTP status codes (410 Gone or 451 Unavailable For Legal Reasons)
  • Makes error handling consistent across implementations (nopfs, Infura etc.)
  • Eliminates string matching in favor of proper error type checking

Related: closes #591

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is bit more to do here, this PR in current form does not solve #591 – see comment inline.

@PsychoPunkSage Mind opening a companion PR against https://github.com/ipfs/kubo that uses boxo from this PR? We have content blocking tests there in https://github.com/ipfs/kubo/blob/master/test/cli/content_blocking_test.go that need to pass before we are comfortable merging this.

Also cc'ing @MichaelMure for feedback from his end: does this look like useful direction?

Comment on lines +59 to +62
// Will default to Error:451 if no status code is provided
if statusCode != http.StatusGone && statusCode != http.StatusUnavailableForLegalReasons {
statusCode = http.StatusUnavailableForLegalReasons // 451 by default
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: legal one should not be the default, switch to 410

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 298 to +302
func isErrContentBlocked(err error) bool {
// TODO: we match error message to avoid pulling nopfs as a dependency
// Ref. https://github.com/ipfs-shipyard/nopfs/blob/cde3b5ba964c13e977f4a95f3bd8ca7d7710fbda/status.go#L87-L89
return strings.Contains(err.Error(), "blocked and cannot be provided")
var blocked *ErrorContentBlocked
return errors.As(err, &blocked)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now nothing will return ErrorContentBlocked type, so isErrContentBlocked will always return false and break content blocking response code in projects that use boxo/gateway, like Kubo.

What is the plan for making this work?

We don't want to depend on https://github.com/ipfs-shipyard/nopfs as noted in the comment here. Perhaps we want to keep the string.Contains and check it in addition to the type check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@lidel lidel added need/author-input Needs input from the original author need/community-input Needs input from the wider community labels Feb 26, 2025
@lidel lidel removed their assignment Feb 26, 2025
@PsychoPunkSage PsychoPunkSage requested a review from lidel March 10, 2025 06:28
@gammazero gammazero marked this pull request as draft March 11, 2025 15:24
@lidel
Copy link
Member

lidel commented Mar 18, 2025

Triage notes:

  • marking requested changes as "resolved" and "done" but not addressing them
  • closing due to the lack of engagement and suspicion this was generated with AI
    • if this was not the case, and you simply were afk for prolonged amount of time, feel free to open new PR (once feedback is addressed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/author-input Needs input from the original author need/community-input Needs input from the wider community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gateway: dedicated error type for denylists
2 participants