Skip to content

Commit

Permalink
Fixed: incorrect message references in some notes
Browse files Browse the repository at this point in the history
Fixes #343.
  • Loading branch information
mnot committed Dec 2, 2023
1 parent f148262 commit 8849d86
Show file tree
Hide file tree
Showing 12 changed files with 39 additions and 35 deletions.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ classifiers = [
"License :: OSI Approved :: MIT License"
]
dependencies = [
"httplint >= 2023.11.1",
"httplint >= 2023.12.1",
"importlib_resources",
"Jinja2 >= 3.1.2",
"markdown >= 3.4.4",
Expand Down
2 changes: 1 addition & 1 deletion redbot/formatter/har.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def format_headers(hdrs: StrHeaderListType) -> List[Dict[str, str]]:

def format_notes(self, resource: HttpResource) -> List[Dict[str, str]]:
out = []
for note in resource.notes:
for note in resource.response.notes:
msg = {
"note_id": note.__class__.__name__,
"subject": note.subject,
Expand Down
7 changes: 5 additions & 2 deletions redbot/formatter/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,11 @@ def format_subrequest_messages(self, category: categories) -> Markup:
)
smsgs = [
note
for note in getattr(self.resource.subreqs[check_name], "notes", [])
if note.level in [levels.BAD] and note not in self.resource.notes
for note in getattr(
self.resource.subreqs[check_name].response, "notes", []
)
if note.level in [levels.BAD]
and note not in self.resource.response.notes
]
if len(smsgs) == 1:
out.append(f" - {len(smsgs)} problem\n")
Expand Down
2 changes: 1 addition & 1 deletion redbot/formatter/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def format_recommendations(self, resource: HttpResource) -> List:
def format_recommendation(
self, resource: HttpResource, category: categories
) -> Union[Dict, None]:
notes = [note for note in resource.notes if note.category == category]
notes = [note for note in resource.response.notes if note.category == category]
if not notes:
return None
out = []
Expand Down
2 changes: 1 addition & 1 deletion redbot/formatter/templates/response_finish.html
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
<span class='help right hidden'>These notes explain what REDbot has found about your URL; hover over each one for a detailed explanation.</span>

{% for category in formatter.note_categories %}
{% for note in resource.notes if note.category == category %}
{% for note in resource.response.notes if note.category == category %}
{% if loop.first %}
<h3>{{ category.value }}{{ category|subrequest_messages }}</h3>
<ul>
Expand Down
2 changes: 1 addition & 1 deletion redbot/formatter/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def format_recommendations(self, resource: HttpResource) -> str:
def format_recommendation(
self, resource: HttpResource, category: categories
) -> str:
notes = [note for note in resource.notes if note.category == category]
notes = [note for note in resource.response.notes if note.category == category]
if not notes:
return ""
out = []
Expand Down
14 changes: 8 additions & 6 deletions redbot/resource/active_check/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ def add_base_note(
self, subject: str, note: Type[Note], **kw: Union[str, int]
) -> None:
"Add a Note to the base resource."
kw["response"] = self.response_phrase
self.base.notes.add(subject, note, **kw)
kw["message"] = self.response_phrase
self.base.response.notes.add(subject, note, **kw)

def check_missing_hdrs(self, hdrs: List[str], note: Type[Note]) -> None:
"""
Expand All @@ -87,19 +87,21 @@ def check_missing_hdrs(self, hdrs: List[str], note: Type[Note]) -> None:
):
missing_hdrs.append(hdr)
if missing_hdrs:
self.base.notes.add("headers", note, missing_hdrs=", ".join(missing_hdrs))
self.notes.add("headers", note, missing_hdrs=", ".join(missing_hdrs))
self.add_base_note("headers", note, missing_hdrs=", ".join(missing_hdrs))
self.response.notes.add(
"headers", note, missing_hdrs=", ".join(missing_hdrs)
)


class MISSING_HDRS_304(Note):
category = categories.VALIDATION
level = levels.WARN
_summary = "%(response)s is missing required headers."
_summary = "%(message)s is missing required headers."
_text = """\
HTTP requires `304 Not Modified` responses to have certain headers, if they are also present in a
normal (e.g., `200 OK` response).
%(response)s is missing the following headers: `%(missing_hdrs)s`.
%(message)s is missing the following headers: `%(missing_hdrs)s`.
This can affect cache operation; because the headers are missing, caches might remove them from
their cached copies."""
6 changes: 2 additions & 4 deletions redbot/resource/active_check/conneg.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,12 @@ class CONNEG_NO_GZIP(Note):
class CONNEG_NO_VARY(Note):
category = categories.CONNEG
level = levels.BAD
_summary = (
"%(response)s is negotiated, but doesn't have an appropriate Vary header."
)
_summary = "%(message)s is negotiated, but doesn't have an appropriate Vary header."
_text = """\
All content negotiated responses need to have a `Vary` header that reflects the header(s) used to
select the response.
%(response)s was negotiated for `gzip` content encoding, so the `Vary` header needs to contain
%(message)s was negotiated for `gzip` content encoding, so the `Vary` header needs to contain
`Accept-Encoding`, the request header used."""


Expand Down
2 changes: 1 addition & 1 deletion redbot/resource/active_check/etag_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
class ETagValidate(SubRequest):
"If an ETag is present, see if it will validate."
check_name = "ETag Validation"
response_phrase = "The 304 response"
response_phrase = "The ETag validation response"

def modify_request_headers(
self, base_headers: StrHeaderListType
Expand Down
2 changes: 1 addition & 1 deletion redbot/resource/active_check/lm_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
class LmValidate(SubRequest):
"If Last-Modified is present, see if it will validate."
check_name = "Last-Modified Validation"
response_phrase = "The 304 response"
response_phrase = "The Last-Modified validation response"
_weekdays = ["Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"]
_months = [
None,
Expand Down
6 changes: 3 additions & 3 deletions redbot/resource/active_check/range.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class RANGE_SUBREQ_PROBLEM(Note):
class UNKNOWN_RANGE(Note):
category = categories.RANGE
level = levels.WARN
_summary = "%(response)s advertises support for non-standard range-units."
_summary = "%(message)s advertises support for non-standard range-units."
_text = """\
The `Accept-Ranges` response header tells clients what `range-unit`s a resource is willing to
process in future requests. HTTP only defines two: `bytes` and `none`.
Expand Down Expand Up @@ -225,12 +225,12 @@ class RANGE_NEG_MISMATCH(Note):
class MISSING_HDRS_206(Note):
category = categories.VALIDATION
level = levels.WARN
_summary = "%(response)s is missing required headers."
_summary = "%(message)s is missing required headers."
_text = """\
HTTP requires `206 Partial Content` responses to have certain headers, if they are also present in
a normal (e.g., `200 OK` response).
%(response)s is missing the following headers: `%(missing_hdrs)s`.
%(message)s is missing the following headers: `%(missing_hdrs)s`.
This can affect cache operation; because the headers are missing, caches might remove them from
their stored copies."""
27 changes: 14 additions & 13 deletions redbot/resource/fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from typing import Any, Dict, List, Tuple, Callable

from httplint import HttpRequestLinter, HttpResponseLinter
from httplint.note import Notes, Note, categories, levels
from httplint.note import Note, categories, levels
from netaddr import IPAddress # type: ignore
import thor
from thor.http.client import HttpClientExchange
Expand Down Expand Up @@ -55,7 +55,6 @@ class RedFetcher(thor.events.EventEmitter):
def __init__(self, config: SectionProxy) -> None:
thor.events.EventEmitter.__init__(self)
self.config = config
self.notes = Notes()
self.transfer_in = 0
self.transfer_out = 0
self.request_content: bytes
Expand All @@ -68,7 +67,7 @@ def __init__(self, config: SectionProxy) -> None:

self.request = HttpRequestLinter()
self.nonfinal_responses: List[HttpResponseLinter] = []
self.response = HttpResponseLinter(notes=self.notes)
self.response = HttpResponseLinter(message_ref=self.response_phrase)
self.response.decoded.processors.append(self.sample_decoded)
self.exchange: HttpClientExchange = None
self.fetch_started = False
Expand Down Expand Up @@ -188,7 +187,7 @@ def _response_nonfinal(
self, status: bytes, phrase: bytes, res_headers: RawHeaderListType
) -> None:
"Got a non-final response."
nfres = HttpResponseLinter(notes=self.notes)
nfres = HttpResponseLinter(message_ref="A non-final response")
nfres.process_response_topline(self.exchange.res_version, status, phrase)
nfres.process_headers(res_headers)
nfres.finish_content(True)
Expand Down Expand Up @@ -247,16 +246,18 @@ def _response_error(self, error: httperr.HttpError) -> None:
err_sample = error.detail[:40] or ""
if isinstance(error, httperr.ExtraDataError):
if self.response.status_code == 304:
self.notes.add("body", BODY_NOT_ALLOWED, sample=err_sample)
self.response.notes.add("body", BODY_NOT_ALLOWED, sample=err_sample)
else:
self.notes.add("body", EXTRA_DATA, sample=err_sample)
self.response.notes.add("body", EXTRA_DATA, sample=err_sample)
elif isinstance(error, httperr.ChunkError):
self.notes.add(
self.response.notes.add(
"header-transfer-encoding", BAD_CHUNK, chunk_sample=err_sample
)
elif isinstance(error, httperr.HeaderSpaceError):
subject = f"header-{error.detail.lower().strip()}"
self.notes.add(subject, HEADER_NAME_SPACE, header_name=error.detail)
self.response.notes.add(
subject, HEADER_NAME_SPACE, header_name=error.detail
)
else:
self.fetch_error = error
self._fetch_done()
Expand All @@ -272,12 +273,12 @@ def _fetch_done(self) -> None:
class BODY_NOT_ALLOWED(Note):
category = categories.CONNECTION
level = levels.BAD
_summary = "%(response)s has content."
_summary = "%(message)s has content."
_text = """\
HTTP defines a few special situations where a response does not allow content. This includes 101,
204 and 304 responses, as well as responses to the `HEAD` method.
%(response)s had data after the headers ended, despite it being disallowed. Clients receiving it
%(message)s had data after the headers ended, despite it being disallowed. Clients receiving it
may treat the content as the next response in the connection, leading to interoperability and
security issues.
Expand All @@ -290,7 +291,7 @@ class BODY_NOT_ALLOWED(Note):
class EXTRA_DATA(Note):
category = categories.CONNECTION
level = levels.BAD
_summary = "%(response)s has extra data after it."
_summary = "%(message)s has extra data after it."
_text = """\
The server sent data after the message ended. This can be caused by an incorrect `Content-Length`
header, or by a programming error in the server itself.
Expand All @@ -304,7 +305,7 @@ class EXTRA_DATA(Note):
class BAD_CHUNK(Note):
category = categories.CONNECTION
level = levels.BAD
_summary = "%(response)s has chunked encoding errors."
_summary = "%(message)s has chunked encoding errors."
_text = """\
The response indicates it uses HTTP chunked encoding, but there was a problem decoding the
chunking.
Expand All @@ -327,7 +328,7 @@ class BAD_CHUNK(Note):
class HEADER_NAME_SPACE(Note):
category = categories.CONNECTION
level = levels.BAD
_summary = "%(response)s has whitespace at the end of the '%(header_name)s' header field name."
_summary = "%(message)s has whitespace at the end of the '%(header_name)s' header field name."
_text = """\
HTTP specifically bans whitespace between header field names and the colon, because they can easily
be confused by recipients; some will strip it, and others won't, leading to a variety of attacks.
Expand Down

0 comments on commit 8849d86

Please sign in to comment.