Skip to content
This repository has been archived by the owner on Mar 29, 2022. It is now read-only.

Commit

Permalink
Refactor the directives mechanism
Browse files Browse the repository at this point in the history
We were using it for headers (Forwarded, Alt-Svc) that were not properly
a list of directives at the top level, leading to some confusion. Also,
the hardcoding of ``argument = required`` for ``Forwarded`` was completely
unnecessary.
  • Loading branch information
vfaronov committed Aug 1, 2017
1 parent fa81887 commit 63ddb24
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 55 deletions.
48 changes: 27 additions & 21 deletions httpolice/header.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,31 +308,39 @@ def __iter__(self):
return iter(v for v in self.value if okay(v))


class DirectivesView(HeaderView): # pylint: disable=abstract-method
class ArgumentsView(HeaderView): # pylint: disable=abstract-method

"""Wraps a header whose parsed value is a list of directives."""
"""
For a header whose parsed value contains some sort of ``name[=argument]``
pairs, and the argument needs to be parsed depending on the name.
"""

knowledge = None

def _process_parsed(self, entry, parsed):
return [self._process_directive(entry, d) for d in parsed]
return [self._process_pair(entry, d) for d in parsed]

def _process_directive(self, entry, directive_with_argument):
directive, argument = directive_with_argument
def _process_pair(self, entry, pair):
name, argument = pair
if argument is None:
if self.knowledge.argument_required(directive):
self.message.complain(1156, entry=entry, directive=directive)
if self.knowledge.argument_required(name):
self.message.complain(1156, entry=entry, name=name)
argument = Unavailable(u'')
else:
syntax = self.knowledge.syntax_for(directive)
if self.knowledge.no_argument(directive):
self.message.complain(1157, entry=entry, directive=directive)
syntax = self.knowledge.syntax_for(name)
if self.knowledge.no_argument(name):
self.message.complain(1157, entry=entry, name=name)
argument = Unavailable(argument)
elif syntax is not None:
argument = parse(argument, syntax,
self.message.complain, 1158, place=entry,
directive=directive, value=argument)
return Parametrized(directive, argument)
name=name, value=argument)
return Parametrized(name, argument)


class DirectivesView(ArgumentsView): # pylint: disable=abstract-method

"""For a header whose value is a list of ``directive[=argument]`` pairs."""

def __getattr__(self, name):
return self[getattr(self.knowledge.accessor, name)]
Expand All @@ -350,8 +358,8 @@ class CacheControlView(DirectivesView, MultiHeaderView):
name = h.cache_control
knowledge = known.cache_directive

def _process_directive(self, entry, directive_with_argument):
(directive, argument) = directive_with_argument
def _process_pair(self, entry, pair):
(directive, argument) = pair
if argument is not None:
(symbol, argument) = argument

Expand All @@ -364,7 +372,7 @@ def _process_directive(self, entry, directive_with_argument):
self.message.complain(1155, directive=directive)

return super(CacheControlView, self). \
_process_directive(entry, (directive, argument))
_process_pair(entry, (directive, argument))


@HeadersView.special_case
Expand All @@ -375,7 +383,7 @@ class StrictTransportSecurityView(DirectivesView, SingleHeaderView):


@HeadersView.special_case
class AltSvcView(DirectivesView, MultiHeaderView):
class AltSvcView(ArgumentsView, MultiHeaderView):

name = h.alt_svc
knowledge = known.alt_svc_param
Expand Down Expand Up @@ -403,8 +411,8 @@ class PreferView(DirectivesView, MultiHeaderView):
# (but they are still part of ``self.value``).

def _process_parsed(self, entry, parsed):
return [Parametrized(self._process_directive(entry, d), params)
for (d, params) in parsed]
return [Parametrized(self._process_pair(entry, pair), params)
for (pair, params) in parsed]

def __getitem__(self, key):
for (preference, value) in self.without_params:
Expand All @@ -425,7 +433,7 @@ class PreferenceAppliedView(DirectivesView, MultiHeaderView):


@HeadersView.special_case
class ForwardedView(DirectivesView, MultiHeaderView):
class ForwardedView(ArgumentsView, MultiHeaderView):

name = h.forwarded
knowledge = known.forwarded_param
Expand All @@ -449,5 +457,3 @@ def _process_parsed(self, entry, parsed):
n_elements=len(elements))

return elements

__getattr__ = None
2 changes: 1 addition & 1 deletion httpolice/known/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Syntax-related fields
to get the ``httpolice.parse.Symbol`` that can parse this syntax.

``argument``
Whether a directive can or must have an argument.
Whether the directive/parameter can or must have an argument.


``header.csv``
Expand Down
30 changes: 8 additions & 22 deletions httpolice/known/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@ class Argument(Enum):
optional = 1
required = 2

class DirectiveKnowledge(SyntaxKnowledge):
class ArgumentKnowledge(SyntaxKnowledge):

"""Knowledge about directives that can have arguments with some syntax."""
"""Knowledge about things that can have arguments with some syntax."""

argument = Argument

Expand Down Expand Up @@ -281,7 +281,7 @@ class ArgumentForm(Enum):
token = 1
quoted_string = 2

class CacheDirectiveKnowledge(DirectiveKnowledge):
class CacheDirectiveKnowledge(ArgumentKnowledge):

argument_form = ArgumentForm

Expand All @@ -302,22 +302,8 @@ def is_for_response(self, key):
cache = cache_directive.accessor


class ForwardedParamKnowledge(DirectiveKnowledge):

def process(self, raw):
# All forwarded parameters require an argument.
processed = super(ForwardedParamKnowledge, self).process(raw)
processed['argument'] = Argument.required
return processed

def unprocess(self, processed, orig_raw): # pragma: no cover
processed = processed.copy()
del processed['argument']
return super(ForwardedParamKnowledge, self).unprocess(processed,
orig_raw)

forwarded_param = ForwardedParamKnowledge(structure.ForwardedParam,
'forwarded_param')
forwarded_param = ArgumentKnowledge(structure.ForwardedParam,
'forwarded_param')
forwarded = forwarded_param.accessor


Expand Down Expand Up @@ -350,7 +336,7 @@ def is_library(self, key):
product = ProductKnowledge(structure.ProductName, 'product')


alt_svc_param = DirectiveKnowledge(structure.AltSvcParam, 'alt_svc_param')
alt_svc_param = ArgumentKnowledge(structure.AltSvcParam, 'alt_svc_param')
altsvc = alt_svc_param.accessor


Expand All @@ -362,11 +348,11 @@ def is_library(self, key):
cc = content_coding.accessor


hsts_directive = DirectiveKnowledge(structure.HSTSDirective, 'hsts_directive')
hsts_directive = ArgumentKnowledge(structure.HSTSDirective, 'hsts_directive')
hsts = hsts_directive.accessor


preference = DirectiveKnowledge(structure.Preference, 'preference')
preference = ArgumentKnowledge(structure.Preference, 'preference')
prefer = preference.accessor


Expand Down
10 changes: 5 additions & 5 deletions httpolice/known/forwarded_param.csv
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
key,title,comment,no_sync,rfc,rfc_section,rfc_appendix,cite_url,cite_title,syntax_module,syntax_symbol,description
by,,,,7239,5.1,,,,rfc7239,node,IP-address of incoming interface of a proxy
for,,,,7239,5.2,,,,rfc7239,node,IP-address of client making a request through a proxy
host,,,,7239,5.3,,,,rfc7230,Host,Host header field of the incoming request
proto,,,,7239,5.4,,,,rfc3986,scheme,Application protocol used for incoming request
key,title,comment,no_sync,rfc,rfc_section,rfc_appendix,cite_url,cite_title,syntax_module,syntax_symbol,argument,description
by,,,,7239,5.1,,,,rfc7239,node,required,IP-address of incoming interface of a proxy
for,,,,7239,5.2,,,,rfc7239,node,required,IP-address of client making a request through a proxy
host,,,,7239,5.3,,,,rfc7230,Host,required,Host header field of the incoming request
proto,,,,7239,5.4,,,,rfc3986,scheme,required,Application protocol used for incoming request
12 changes: 6 additions & 6 deletions httpolice/notices.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1174,18 +1174,18 @@ One non-obvious thing is how references work
</comment>

<error id="1156">
<title><var ref="entry"/>: <var ref="directive"/> needs an argument</title>
<explain>According to the specification, the <var ref="directive"/> directive must have an argument. But in this message’s <var ref="entry"/> header, it has no argument.</explain>
<title><var ref="entry"/>: <var ref="name"/> needs an argument</title>
<explain>According to the specification, <var ref="name"/>’ in <var ref="entry"/> must have an argument. But in this message, it doesn’t.</explain>
</error>

<error id="1157">
<title><var ref="entry"/>: <var ref="directive"/> can’t have an argument</title>
<explain>In this message’s <var ref="entry"/> header, the <var ref="directive"/> directive has an argument. But the specification defines no argument for this directive.</explain>
<title><var ref="entry"/>: <var ref="name"/> can’t have an argument</title>
<explain>In this message’s <var ref="entry"/> header, <var ref="name"/>has an argument. But the specification defines no argument for ‘<var ref="name"/>’.</explain>
</error>

<error id="1158">
<title>Syntax error in <var ref="place"/>: <var ref="directive"/></title>
<explain>“<var ref="value"/>” is not a correct value for a <var ref="directive"/> directive in <var ref="place"/>.</explain>
<title>Syntax error in <var ref="place"/>: <var ref="name"/></title>
<explain>“<var ref="value"/>” is not a correct value for <var ref="name"/> in <var ref="place"/>.</explain>
<exception/>
</error>

Expand Down

0 comments on commit 63ddb24

Please sign in to comment.