diff --git a/README.md b/README.md
index e9eee9c..de7fae2 100644
--- a/README.md
+++ b/README.md
@@ -103,8 +103,8 @@ complete and are not a legal assessment.
- Criteria with `check.type: links` check every `RelatedResource` of the
passport for the required attributes and send a HEAD request to its URL
(at most 20 URLs per passport). A URL that does not answer gives a warning.
-- Criteria with `check.type: html` (DPP-SEC-014, DPP-SEC-021) request the
- product identifier with `Accept: text/html`. Without an HTML view the result
+- Criteria with `check.type: html` (DPP-SEC-014, DPP-SEC-021, DPP-DAT-020)
+ request the product identifier with `Accept: text/html`. Without an HTML view the result
is `skipped` (`no_evidence`).
- With `external_resources: forbidden` dpplint lists the URLs a browser loads
without user action while it displays the HTML view (scripts, style
@@ -121,6 +121,13 @@ complete and are not a legal assessment.
elements. Every finding fails the criterion. Code assembled at run time,
obfuscated or loaded by a script, and scripts from other hosts, are not
seen; an interface name in a comment or string counts.
+ - With `markup_errors: forbidden` dpplint posts the HTML view, as received
+ and with the media type and charset of the response, to the Nu Html
+ Checker that it runs itself (`NU_CHECKER_URL`; see "Deployment"). It never
+ sends the document to a public instance. Every message of type `error`
+ fails the criterion; warnings and other `info` messages do not count.
+ Without a checker, or if the checker does not answer or cannot check the
+ document, the criterion is `skipped` (`not_evaluated`).
- Criteria with `check.type: element_ids` (DPP-DAT-017, DPP-DAT-021) check
that elementIds are unique. In the expanded full serialization dpplint
compares the elementIds within every array of data elements (`unique:
@@ -167,6 +174,16 @@ default is `main`. Branch and tag names are resolved to their commit first, so
that the Docker build cache never reuses an older dpp-criteria. `GET /version` shows the commit in use. The image is built
for linux/amd64, like the SOyA web-cli it contains.
+Without `NU_CHECKER_URL` the criteria that need the Nu Html Checker are
+skipped. To run them locally, start the checker on a network of its own next
+to dpplint:
+
+```
+docker network create dpplint
+docker run --rm -d --name nu-html-checker --network dpplint ghcr.io/validator/validator@sha256:8f21030474e68e06905cfa801b37376e99c0e37867c653c5b6cd7e2dcaf3f503
+docker run --rm --platform linux/amd64 --network dpplint -p 3000:3000 -e NU_CHECKER_URL=http://nu-html-checker:8888/ oydeu/dpplint
+```
+
## Deployment
`kubernetes/` holds the manifests for dpplint.ownyourdata.eu: deployment,
@@ -183,6 +200,15 @@ answer, fails its readiness probe (`/ready`) and is taken out of the ingress
until it answers again; the liveness probe only checks that the port is open,
so a busy replica is not restarted.
+Each pod also runs the Nu Html Checker (`ghcr.io/validator/validator`, MIT
+licence) as a second container, named by its digest like the dpplint image.
+It listens on 127.0.0.1:8888 only, so it is reachable from dpplint in the same
+pod and from nowhere else, and it runs with a heap of 192 MB within a memory
+limit of 512 Mi. dpplint finds it through `NU_CHECKER_URL`. The pod's
+readiness does not depend on it: if the checker is down or still starting,
+only the criteria that need it are skipped. Its liveness probe restarts it if
+it stops answering.
+
The deployment names the image by its digest (`oydeu/dpplint@sha256:…`), so
that all replicas run the same image and a restart never picks up a newer
one. The digest of a new image gets into the manifest when the image is
diff --git a/app/services/html_check.rb b/app/services/html_check.rb
index aaf00bb..8c07713 100644
--- a/app/services/html_check.rb
+++ b/app/services/html_check.rb
@@ -20,6 +20,12 @@
# the first MAX_SCRIPT_BYTES of each; a script that cannot be retrieved is not
# checked), and in the markup for frames granted device permissions
# (DEVICE_PERMISSIONS), file inputs with capture and permission elements.
+#
+# markup_errors: forbidden sends the document as received to the Nu Html
+# Checker that dpplint runs itself (NuChecker) and reports every message of
+# type error; warnings and other info messages are not counted. Without a
+# checker, or if it cannot check the document, the criterion is skipped
+# (not_evaluated).
class HtmlCheck
Outcome = Struct.new(:skipped, :code, :messages, keyword_init: true)
@@ -44,6 +50,7 @@ class HtmlCheck
MAX_SCRIPTS = 10
MAX_SCRIPT_BYTES = 2 * 1024 * 1024
MAX_FINDINGS = 10
+ MAX_MESSAGE_LENGTH = 300
SRC_ELEMENTS = %w[script img iframe frame embed audio video source track].freeze
SRCSET_ELEMENTS = %w[img source].freeze
@@ -53,10 +60,11 @@ class HtmlCheck
CSS_IMPORT = /@import\s+(?:"([^"]*)"|'([^']*)')/i
MAX_LISTED = 10
- def initialize(check, product_id, resolver)
+ def initialize(check, product_id, resolver, nu_checker: nil)
@check = check
@product_id = product_id
@resolver = resolver
+ @nu_checker = nu_checker
end
def outcome
@@ -68,7 +76,10 @@ def outcome
messages = []
messages << external_message(res) if @check["external_resources"] == "forbidden"
messages << device_message(res) if @check["device_features"] == "forbidden"
+ messages << markup_message(res) if @check["markup_errors"] == "forbidden"
Outcome.new(messages: messages.compact)
+ rescue NuChecker::Unavailable => e
+ Outcome.new(skipped: e.message, code: "not_evaluated")
end
# Device interfaces found in a piece of script: [[label, matched text], …].
@@ -220,6 +231,25 @@ def script_findings(doc, page_url)
end
end
+ # Errors of the Nu Html Checker, with their line: "line 12: message".
+ def markup_message(res)
+ checker = @nu_checker || NuChecker.new
+ errors = checker.messages(res.body, res.content_type.presence || "text/html").select { |m| m["type"] == "error" }
+ return nil if errors.empty?
+
+ listed = errors.first(MAX_FINDINGS).map { |m| markup_error(m) }
+ more = errors.size > MAX_FINDINGS ? " and #{errors.size - MAX_FINDINGS} more" : ""
+ { severity: "violation",
+ message: "the Nu Html Checker reports #{errors.size} error(s) in the HTML view: #{listed.join('; ')}#{more}" }
+ end
+
+ def markup_error(message)
+ text = message["message"].to_s.gsub(/\s+/, " ").strip
+ text = "#{text[0, MAX_MESSAGE_LENGTH - 1]}…" if text.length > MAX_MESSAGE_LENGTH
+ line = message["lastLine"] || message["firstLine"]
+ line ? "line #{line}: #{text}" : text
+ end
+
def describe(res)
return res.error if res.error
diff --git a/app/services/nu_checker.rb b/app/services/nu_checker.rb
new file mode 100644
index 0000000..f1f1077
--- /dev/null
+++ b/app/services/nu_checker.rb
@@ -0,0 +1,59 @@
+require "net/http"
+
+# Client for the Nu Html Checker (vnu) that dpplint runs itself (NU_CHECKER_URL;
+# in the deployment a second container of the pod that listens on 127.0.0.1
+# only). The HTML view is posted to it as received; it is never sent to a
+# public instance. Without NU_CHECKER_URL there is no checker. Each call waits
+# at most OPEN_TIMEOUT and READ_TIMEOUT, but never beyond the time limit of
+# the check (CheckDeadline).
+class NuChecker
+ class Unavailable < StandardError; end
+
+ OPEN_TIMEOUT = 2
+ READ_TIMEOUT = 15
+
+ def initialize(base = Rails.configuration.x.dpplint.nu_checker, deadline: CheckDeadline.new)
+ @base = base.to_s.strip
+ @deadline = deadline
+ end
+
+ def configured? = !@base.empty?
+
+ # The messages of the checker for one HTML document, as the checker gives
+ # them ("type", "subType", "message", "lastLine", …). Raises Unavailable if
+ # there is no checker, it does not answer, or it could not check the
+ # document (non-document-error).
+ def messages(body, content_type)
+ raise Unavailable, "no Nu Html Checker is configured for this instance" unless configured?
+
+ uri = URI.join(@base.end_with?("/") ? @base : "#{@base}/", "?out=json")
+ limits = @deadline.split(OPEN_TIMEOUT, READ_TIMEOUT)
+ res = Net::HTTP.start(uri.host, uri.port, open_timeout: limits.connect, read_timeout: limits.read, write_timeout: limits.read) do |http|
+ http.post(uri.request_uri, body.to_s, "Content-Type" => content_type, "Accept" => "application/json")
+ end
+ parse(res)
+ rescue Unavailable, CheckDeadline::Reached
+ raise
+ rescue Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout
+ @deadline.timed_out!(limits)
+ raise Unavailable, "the Nu Html Checker did not answer in time"
+ rescue StandardError => e
+ raise Unavailable, "the Nu Html Checker is not reachable (#{e.class})"
+ end
+
+ private
+
+ def parse(res)
+ raise Unavailable, "the Nu Html Checker answered with HTTP #{res.code}" unless res.is_a?(Net::HTTPSuccess)
+
+ list = JSON.parse(res.body.to_s)["messages"]
+ raise Unavailable, "the Nu Html Checker answered without a list of messages" unless list.is_a?(Array)
+
+ failure = list.find { |m| m.is_a?(Hash) && m["type"] == "non-document-error" }
+ raise Unavailable, "the Nu Html Checker could not check the document: #{failure['message'].to_s.strip}" if failure
+
+ list.grep(Hash)
+ rescue JSON::ParserError
+ raise Unavailable, "the Nu Html Checker answered with invalid JSON"
+ end
+end
diff --git a/app/services/passport_linter.rb b/app/services/passport_linter.rb
index 3c1aecd..ff9babe 100644
--- a/app/services/passport_linter.rb
+++ b/app/services/passport_linter.rb
@@ -23,12 +23,14 @@ class PassportLinter
REASON_CODES = %w[not_applicable not_implemented no_evidence needs_credentials not_sent unreachable not_evaluated].freeze
def initialize(catalogue: CriteriaCatalogue.new, deadline: CheckDeadline.new, web_cli: SoyaWebCli.new(deadline: deadline),
- resolver: HttpResolver.new(deadline: deadline), didlint: Didlint.new(deadline: deadline))
+ resolver: HttpResolver.new(deadline: deadline), didlint: Didlint.new(deadline: deadline),
+ nu_checker: NuChecker.new(deadline: deadline))
@catalogue = catalogue
@deadline = deadline
@web_cli = web_cli
@resolver = resolver
@didlint = didlint
+ @nu_checker = nu_checker
end
def run(passport:, product_id: nil, retrieval: nil, jws: nil, jws_only: false, raw: nil, jws_unchecked: nil, source: nil)
@@ -154,7 +156,7 @@ def resolve(base, check)
def html(base, check)
return skip(base, "needs a product identifier", "not_evaluated") if @product_id.blank?
- outcome = HtmlCheck.new(check, @product_id, @resolver).outcome
+ outcome = HtmlCheck.new(check, @product_id, @resolver, nu_checker: @nu_checker).outcome
return skip(base, outcome.skipped, outcome.code) if outcome.skipped
base.merge(result: result_for(outcome.messages), messages: outcome.messages)
diff --git a/config/initializers/dpplint.rb b/config/initializers/dpplint.rb
index f676e72..75afa46 100644
--- a/config/initializers/dpplint.rb
+++ b/config/initializers/dpplint.rb
@@ -6,6 +6,10 @@
# SOYA_CONTEXT.json records which SOyA context the structures of the image were
# built with (fetched or a cached copy, and from when).
#
+# NU_CHECKER_URL is the Nu Html Checker that dpplint runs itself (in the
+# deployment a second container of the pod, on 127.0.0.1); without it,
+# criteria that need the checker are skipped.
+#
# A check of the API ends after at most CHECK_DEADLINE seconds (default 35),
# with the criteria evaluated so far; all other limits are cut to the time
# left. The start page waits for a check at most CHECK_TIMEOUT seconds
@@ -16,6 +20,7 @@
c.didlint = ENV.fetch("DIDLINT_URL", "https://didlint.ownyourdata.eu")
c.didlint_timeout = ENV.fetch("DIDLINT_TIMEOUT", "10").to_f
c.didlint_budget = ENV.fetch("DIDLINT_BUDGET", "20").to_f
+ c.nu_checker = ENV.fetch("NU_CHECKER_URL", "")
c.check_deadline = ENV.fetch("CHECK_DEADLINE", "35").to_f
c.check_timeout = ENV.fetch("CHECK_TIMEOUT", (c.check_deadline + 5).to_s).to_f
c.criteria_dir = ENV.fetch("DPP_CRITERIA_DIR", Rails.root.join("criteria").to_s)
diff --git a/kubernetes/deployment.yaml b/kubernetes/deployment.yaml
index 8fe73e2..ca583ff 100644
--- a/kubernetes/deployment.yaml
+++ b/kubernetes/deployment.yaml
@@ -26,7 +26,7 @@ spec:
app: dpplint
containers:
- name: dpplint
- image: oydeu/dpplint@sha256:c7cc08a926cf261d3fd3321aaa851343118f3e810ebd0a763312ec21050e1db8
+ image: oydeu/dpplint@sha256:30f7b99950874ba357455baf42e68ad96968db9f65246e73f559300904f07414
imagePullPolicy: IfNotPresent
ports:
- containerPort: 3000
@@ -39,6 +39,8 @@ spec:
value: https://didlint.ownyourdata.eu
- name: RAILS_MAX_THREADS
value: "16"
+ - name: NU_CHECKER_URL
+ value: http://127.0.0.1:8888/
resources:
requests:
cpu: 100m
@@ -60,3 +62,29 @@ spec:
periodSeconds: 30
timeoutSeconds: 5
failureThreshold: 3
+ - name: nu-html-checker
+ image: ghcr.io/validator/validator@sha256:8f21030474e68e06905cfa801b37376e99c0e37867c653c5b6cd7e2dcaf3f503
+ imagePullPolicy: IfNotPresent
+ env:
+ - name: BIND_ADDRESS
+ value: 127.0.0.1
+ - name: JAVA_TOOL_OPTIONS
+ value: -Xmx192m -Xss512k
+ resources:
+ requests:
+ cpu: 50m
+ memory: 320Mi
+ limits:
+ memory: 512Mi
+ livenessProbe:
+ exec:
+ command:
+ - wget
+ - --no-verbose
+ - --tries=1
+ - --spider
+ - http://127.0.0.1:8888/
+ initialDelaySeconds: 30
+ periodSeconds: 30
+ timeoutSeconds: 5
+ failureThreshold: 3
diff --git a/test/fixtures/files/html/DPP-DAT-020/README.md b/test/fixtures/files/html/DPP-DAT-020/README.md
new file mode 100644
index 0000000..27cefe0
--- /dev/null
+++ b/test/fixtures/files/html/DPP-DAT-020/README.md
@@ -0,0 +1,22 @@
+# Test vectors for DPP-DAT-020
+
+`valid/` and `invalid/` hold HTML views; the files in `valid/` have no error
+in the Nu Html Checker, every file in `invalid/` has at least one.
+`invalid/legacy-encoding.html` is encoded in windows-1252 and is served with
+that charset; all others are UTF-8.
+
+`checker/` holds the answers of the Nu Html Checker 26.10.12 (the image
+`ghcr.io/validator/validator` that `kubernetes/deployment.yaml` names) to
+these files, posted the way dpplint posts them. The tests answer with them
+instead of running the checker. To record them again, start the checker and
+post every file:
+
+```
+docker run --rm -d --name vnu -p 127.0.0.1:8888:8888 ghcr.io/validator/validator@sha256:8f21030474e68e06905cfa801b37376e99c0e37867c653c5b6cd7e2dcaf3f503
+for f in valid/*.html invalid/*.html; do
+ n=$(basename "$f" .html); cs=utf-8; [ "$n" = legacy-encoding ] && cs=windows-1252
+ curl -s -H "Content-Type: text/html; charset=$cs" --data-binary @"$f" \
+ "http://127.0.0.1:8888/?out=json" | python3 -m json.tool --no-ensure-ascii > "checker/$(dirname "$f")-$n.json"
+done
+docker stop vnu
+```
diff --git a/test/fixtures/files/html/DPP-DAT-020/checker/invalid-css-error-in-style-attribute.json b/test/fixtures/files/html/DPP-DAT-020/checker/invalid-css-error-in-style-attribute.json
new file mode 100644
index 0000000..25043e4
--- /dev/null
+++ b/test/fixtures/files/html/DPP-DAT-020/checker/invalid-css-error-in-style-attribute.json
@@ -0,0 +1,15 @@
+{
+ "version": "26.10.12 (973e40b)",
+ "messages": [
+ {
+ "type": "error",
+ "lastLine": 9,
+ "lastColumn": 26,
+ "firstColumn": 1,
+ "message": "CSS: “color”: “bluish” is not a “color” value.",
+ "extract": "y>\nLamp A",
+ "hiliteStart": 10,
+ "hiliteLength": 26
+ }
+ ]
+}
diff --git a/test/fixtures/files/html/DPP-DAT-020/checker/invalid-duplicate-id.json b/test/fixtures/files/html/DPP-DAT-020/checker/invalid-duplicate-id.json
new file mode 100644
index 0000000..e4af845
--- /dev/null
+++ b/test/fixtures/files/html/DPP-DAT-020/checker/invalid-duplicate-id.json
@@ -0,0 +1,26 @@
+{
+ "version": "26.10.12 (973e40b)",
+ "messages": [
+ {
+ "type": "error",
+ "lastLine": 10,
+ "lastColumn": 22,
+ "firstColumn": 1,
+ "message": "Duplicate ID “product”.",
+ "extract": " A60
\nE",
+ "hiliteStart": 10,
+ "hiliteLength": 22
+ },
+ {
+ "type": "info",
+ "lastLine": 9,
+ "lastColumn": 17,
+ "firstColumn": 1,
+ "subType": "warning",
+ "message": "The first occurrence of ID “product” was here.",
+ "extract": "y>\n
Lamp A",
+ "hiliteStart": 10,
+ "hiliteLength": 17
+ }
+ ]
+}
diff --git a/test/fixtures/files/html/DPP-DAT-020/checker/invalid-legacy-encoding.json b/test/fixtures/files/html/DPP-DAT-020/checker/invalid-legacy-encoding.json
new file mode 100644
index 0000000..9dd8361
--- /dev/null
+++ b/test/fixtures/files/html/DPP-DAT-020/checker/invalid-legacy-encoding.json
@@ -0,0 +1,19 @@
+{
+ "version": "26.10.12 (973e40b)",
+ "messages": [
+ {
+ "type": "error",
+ "message": "Legacy encoding “windows-1252” used. Documents must use UTF-8."
+ },
+ {
+ "type": "error",
+ "lastLine": 4,
+ "lastColumn": 29,
+ "firstColumn": 1,
+ "message": "The only allowed value for the “charset” attribute for the “meta” element is “utf-8”.",
+ "extract": "\">\n\n\n
Rated power: 750 W
+x", checker: checker).messages.first[:message] + assert_match(/\Athe Nu Html Checker reports 12 error\(s\) in the HTML view: line 1: Error 1\.; /, message) + assert_match(/line 10: Error 10\. and 2 more\z/, message) + end + + test "without a configured checker the criterion is skipped" do + o = outcome("
x", checker: NuChecker.new("")) + assert_equal "not_evaluated", o.code + assert_equal "no Nu Html Checker is configured for this instance", o.skipped + end + + test "a checker that cannot be reached, answers with an error page or cannot check the document skips the criterion" do + gone = NuChecker.new("http://127.0.0.1:9/") + assert_match(/\Athe Nu Html Checker is not reachable/, outcome("
x", checker: gone).skipped) + assert_equal "the Nu Html Checker answered with HTTP 500", outcome("
x", checker: server { [0, SlowServer.http(500)] }).skipped + assert_equal "the Nu Html Checker answered with invalid JSON", outcome("
x", checker: server { [0, json("")] }).skipped + refused = { "messages" => [{ "type" => "non-document-error", "subType" => "io", "message" => "Non-XML Content-Type: “application/pdf”." }] } + o = outcome("
x", checker: server { [0, json(refused.to_json)] }) + assert_equal "not_evaluated", o.code + assert_equal "the Nu Html Checker could not check the document: Non-XML Content-Type: “application/pdf”.", o.skipped + end + + test "a checker that does not answer ends with the time limit of the check" do + hanging = server { [0, nil] } + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + assert_raises(CheckDeadline::Reached) do + NuChecker.new(hanging.url("/"), deadline: CheckDeadline.new(1)).messages("
x", "text/html") + end + assert_operator Process.clock_gettime(Process::CLOCK_MONOTONIC) - started, :<, 2 + assert hanging.all_given_up? + end + + test "the checker is asked only with the option and only for an HTML view" do + checker = recorded_checker + HtmlCheck.new({ "type" => "html", "device_features" => "forbidden" }, UPI, FakeResolver.new("
x", "text/html"), + nu_checker: NuChecker.new(checker.url("/"))).outcome + o = HtmlCheck.new(CHECK, UPI, FakeResolver.new("{}", "application/json"), nu_checker: NuChecker.new(checker.url("/"))).outcome + assert_equal "no_evidence", o.code + assert_empty checker.requests + end + + test "a base URL without a trailing slash works" do + checker = server { [0, json('{"messages":[]}')] } + assert_equal [], NuChecker.new(checker.url("/").chomp("/")).messages("
x", "text/html") + assert_equal "POST /?out=json HTTP/1.1", checker.requests.last.first + end +end diff --git a/test/services/passport_linter_test.rb b/test/services/passport_linter_test.rb index 735239d..291a92a 100644 --- a/test/services/passport_linter_test.rb +++ b/test/services/passport_linter_test.rb @@ -177,4 +177,21 @@ def lint(responses, check = CHECK) assert_equal "warning", report[:criteria].first[:result] assert_equal "1 of 1 automated checks passed", report[:summary][:text] end + + test "html criteria with markup_errors use the Nu Html Checker of the linter, and are skipped without one" do + check = { "type" => "html", "markup_errors" => "forbidden" } + checker = SlowServer.new { [0, SlowServer.http(200, '{"messages":[{"type":"error","lastLine":3,"message":"Stray end tag “p”."}]}')] } + linter = lambda do |nu| + PassportLinter.new(catalogue: FakeCatalogue.new([criterion(check).merge("id" => "DPP-DAT-020")]), + resolver: FakeResolver.new("text/html" => HTML), nu_checker: nu).run(passport: nil, product_id: UPI) + end + failed = linter.call(NuChecker.new(checker.url("/")))[:criteria].first + assert_equal "failed", failed[:result] + assert_equal "the Nu Html Checker reports 1 error(s) in the HTML view: line 3: Stray end tag “p”.", failed[:messages].first[:message] + skipped = linter.call(NuChecker.new(""))[:criteria].first + assert_equal %w[skipped not_evaluated], skipped.values_at(:result, :reason_code) + assert_equal "no Nu Html Checker is configured for this instance", skipped[:reason] + ensure + checker&.stop + end end diff --git a/test/support_slow_server.rb b/test/support_slow_server.rb index 7d7e942..68b1e30 100644 --- a/test/support_slow_server.rb +++ b/test/support_slow_server.rb @@ -1,9 +1,10 @@ require "socket" # A plain HTTP server on 127.0.0.1 for tests of time limits. The block gets -# the request line and header lines and returns [delay, response]: the -# response is sent after `delay` seconds; nil means the server never answers -# and waits until the client gives up. `open_requests` counts the requests +# the request line and header lines (and the body, if the request has a +# Content-Length) and returns [delay, response]: the response is sent after +# `delay` seconds; nil means the server never answers and waits until the +# client gives up. `open_requests` counts the requests # the client has not given up yet, so a test can see that nothing of a check # keeps running after it. class SlowServer @@ -54,8 +55,10 @@ def serve(client) while (line = c.gets) && line != "\r\n" lines << line.chomp end + length = lines.grep(/\Acontent-length:/i).first.to_s.split(":", 2).last.to_i + body = length.positive? ? c.read(length) : nil @lock.synchronize { @requests << lines } - delay, response = @handler.call(lines) + delay, response = @handler.call(lines, body) if response sleep delay.to_f c.write(response)