From 95a1306e16c219d2704fe07373486035c8fbe498 Mon Sep 17 00:00:00 2001 From: Christoph Fabianek Date: Mon, 26 Oct 2026 14:17:28 +0100 Subject: [PATCH] Check the HTML view with the Nu Html Checker in the same pod html with markup_errors: forbidden (DPP-DAT-020) posts the HTML view, as received and with its media type and charset, to the Nu Html Checker at NU_CHECKER_URL and fails on every message of type error; warnings and other info messages do not count. Without a checker, or if it does not answer or cannot check the document, the criterion is skipped. The deployment runs the checker (ghcr.io/validator/validator, named by its digest) as a second container of each pod. It listens on 127.0.0.1 only and runs with a heap of 192 MB within a memory limit of 512 Mi; the readiness of the pod does not depend on it. Test vectors for DPP-DAT-020 with the recorded answers of the checker are under test/fixtures/files/html/DPP-DAT-020. The image is built with the dpp-criteria commit that adds the criterion. --- README.md | 30 +++- app/services/html_check.rb | 32 +++- app/services/nu_checker.rb | 59 ++++++++ app/services/passport_linter.rb | 6 +- config/initializers/dpplint.rb | 5 + kubernetes/deployment.yaml | 30 +++- .../fixtures/files/html/DPP-DAT-020/README.md | 22 +++ .../invalid-css-error-in-style-attribute.json | 15 ++ .../checker/invalid-duplicate-id.json | 26 ++++ .../checker/invalid-legacy-encoding.json | 19 +++ .../checker/invalid-misnested-paragraph.json | 15 ++ .../checker/invalid-missing-title.json | 15 ++ .../checker/invalid-obsolete-element.json | 15 ++ .../checker/valid-passport-view.json | 4 + .../checker/valid-umlauts-in-utf-8.json | 4 + .../checker/valid-warnings-only.json | 27 ++++ .../invalid/css-error-in-style-attribute.html | 13 ++ .../DPP-DAT-020/invalid/duplicate-id.html | 16 ++ .../DPP-DAT-020/invalid/legacy-encoding.html | 13 ++ .../invalid/misnested-paragraph.html | 15 ++ .../DPP-DAT-020/invalid/missing-title.html | 12 ++ .../DPP-DAT-020/invalid/obsolete-element.html | 13 ++ .../html/DPP-DAT-020/valid/passport-view.html | 30 ++++ .../DPP-DAT-020/valid/umlauts-in-utf-8.html | 17 +++ .../html/DPP-DAT-020/valid/warnings-only.html | 16 ++ test/integration/validations_test.rb | 2 +- test/services/html_check_markup_test.rb | 140 ++++++++++++++++++ test/services/passport_linter_test.rb | 17 +++ test/support_slow_server.rb | 11 +- 29 files changed, 628 insertions(+), 11 deletions(-) create mode 100644 app/services/nu_checker.rb create mode 100644 test/fixtures/files/html/DPP-DAT-020/README.md create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/invalid-css-error-in-style-attribute.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/invalid-duplicate-id.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/invalid-legacy-encoding.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/invalid-misnested-paragraph.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/invalid-missing-title.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/invalid-obsolete-element.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/valid-passport-view.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/valid-umlauts-in-utf-8.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/checker/valid-warnings-only.json create mode 100644 test/fixtures/files/html/DPP-DAT-020/invalid/css-error-in-style-attribute.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/invalid/duplicate-id.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/invalid/legacy-encoding.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/invalid/misnested-paragraph.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/invalid/missing-title.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/invalid/obsolete-element.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/valid/passport-view.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/valid/umlauts-in-utf-8.html create mode 100644 test/fixtures/files/html/DPP-DAT-020/valid/warnings-only.html create mode 100644 test/services/html_check_markup_test.rb 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>\n
\n

Lamp 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

\n
\n

E", + "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
\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\n

\n\n\n\n\n
\n

L", + "hiliteStart": 10, + "hiliteLength": 8 + } + ] +} diff --git a/test/fixtures/files/html/DPP-DAT-020/checker/valid-passport-view.json b/test/fixtures/files/html/DPP-DAT-020/checker/valid-passport-view.json new file mode 100644 index 0000000..6fcfa24 --- /dev/null +++ b/test/fixtures/files/html/DPP-DAT-020/checker/valid-passport-view.json @@ -0,0 +1,4 @@ +{ + "version": "26.10.12 (973e40b)", + "messages": [] +} diff --git a/test/fixtures/files/html/DPP-DAT-020/checker/valid-umlauts-in-utf-8.json b/test/fixtures/files/html/DPP-DAT-020/checker/valid-umlauts-in-utf-8.json new file mode 100644 index 0000000..6fcfa24 --- /dev/null +++ b/test/fixtures/files/html/DPP-DAT-020/checker/valid-umlauts-in-utf-8.json @@ -0,0 +1,4 @@ +{ + "version": "26.10.12 (973e40b)", + "messages": [] +} diff --git a/test/fixtures/files/html/DPP-DAT-020/checker/valid-warnings-only.json b/test/fixtures/files/html/DPP-DAT-020/checker/valid-warnings-only.json new file mode 100644 index 0000000..09ce8b7 --- /dev/null +++ b/test/fixtures/files/html/DPP-DAT-020/checker/valid-warnings-only.json @@ -0,0 +1,27 @@ +{ + "version": "26.10.12 (973e40b)", + "messages": [ + { + "type": "info", + "lastLine": 6, + "lastColumn": 31, + "firstColumn": 1, + "subType": "warning", + "message": "The “type” attribute is unnecessary for JavaScript resources.", + "extract": "l\n + + +
+

Drill

+
+

Rated power: 750 W

+
+
+ + diff --git a/test/integration/validations_test.rb b/test/integration/validations_test.rb index 84776f9..25dbbaa 100644 --- a/test/integration/validations_test.rb +++ b/test/integration/validations_test.rb @@ -127,7 +127,7 @@ def criterion(result, id) = result["criteria"].find { |c| c["id"] == id } test "checks that request the product identifier need one" do result = lint(reference) - %w[DPP-ID-001 DPP-ID-013 DPP-DAT-003 DPP-DAT-016 DPP-SEC-014 DPP-SEC-020 DPP-SEC-021].each do |id| + %w[DPP-ID-001 DPP-ID-013 DPP-DAT-003 DPP-DAT-016 DPP-DAT-020 DPP-SEC-014 DPP-SEC-020 DPP-SEC-021].each do |id| assert_equal "skipped", criterion(result, id)["result"], id end assert_equal "rated by dpp-validator from its daily runs", criterion(result, "DPP-ID-002")["reason"] diff --git a/test/services/html_check_markup_test.rb b/test/services/html_check_markup_test.rb new file mode 100644 index 0000000..e468df3 --- /dev/null +++ b/test/services/html_check_markup_test.rb @@ -0,0 +1,140 @@ +require "test_helper" + +# html with markup_errors: forbidden (DPP-DAT-020): the HTML view is posted to +# the Nu Html Checker; every error fails the criterion, warnings and other +# info messages do not count. The checker here is a local server that gives +# the answers the Nu Html Checker gave to the test vectors (recorded in +# test/fixtures/files/html/DPP-DAT-020/checker, see README.md there). +class HtmlCheckMarkupTest < ActiveSupport::TestCase + UPI = "https://dpp.example.org/01/09520123456788/21/0001".freeze + CHECK = { "type" => "html", "markup_errors" => "forbidden" }.freeze + VECTORS = Rails.root.join("test/fixtures/files/html/DPP-DAT-020") + CHARSETS = { "legacy-encoding" => "windows-1252" }.freeze + + class FakeResolver + attr_reader :requested + + def initialize(page, type, status: 200) = (@page, @type, @status, @requested = page, type, status, []) + + def get(url, accept: nil) + @requested << [url, accept] + HttpResolver::Response.new(url: url, status: @status, content_type: @type, body: @page, headers: {}) + end + end + + setup { @servers = [] } + + teardown { @servers.each(&:stop) } + + def server(&handler) = SlowServer.new(&handler).tap { |s| @servers << s } + + def json(body) = SlowServer.http(200, body, "Content-Type" => "application/json; charset=utf-8") + + # A checker that answers each test vector as recorded. + def recorded_checker + answers = Dir[VECTORS.join("{valid,invalid}", "*.html")].to_h do |file| + [File.binread(file), File.read(VECTORS.join("checker", "#{File.basename(File.dirname(file))}-#{File.basename(file, '.html')}.json"))] + end + server { |_lines, body| [0, answers.key?(body.to_s.b) ? json(answers[body.to_s.b]) : SlowServer.http(500)] } + end + + def content_type(file) = "text/html; charset=#{CHARSETS.fetch(File.basename(file, '.html'), 'utf-8')}" + + def outcome(page, type: "text/html; charset=utf-8", checker: recorded_checker, check: CHECK) + nu = checker.is_a?(NuChecker) ? checker : NuChecker.new(checker.url("/")) + HtmlCheck.new(check, UPI, FakeResolver.new(page, type), nu_checker: nu).outcome + end + + def vector(name) = VECTORS.join(name) + + test "DPP-DAT-020 test vectors: valid ones pass, invalid ones fail" do + checker = recorded_checker + valid = Dir[VECTORS.join("valid", "*.html")].sort + invalid = Dir[VECTORS.join("invalid", "*.html")].sort + assert_operator valid.size, :>=, 3 + assert_operator invalid.size, :>=, 6 + valid.each do |f| + o = outcome(File.binread(f), type: content_type(f), checker: checker) + assert_nil o.skipped, f + assert_empty o.messages, f + end + invalid.each do |f| + assert_equal ["violation"], outcome(File.binread(f), type: content_type(f), checker: checker).messages.map { |m| m[:severity] }, f + end + end + + test "the HTML view is posted as received, with its media type and charset" do + checker = recorded_checker + page = File.binread(vector("invalid/legacy-encoding.html")) + outcome(page, type: "text/html; charset=windows-1252", checker: checker) + lines = checker.requests.last + assert_equal "POST /?out=json HTTP/1.1", lines.first + assert_includes lines, "Content-Type: text/html; charset=windows-1252" + assert_includes lines, "Accept: application/json" + end + + test "findings name the line and the message of each error" do + assert_equal "the Nu Html Checker reports 1 error(s) in the HTML view: line 10: Duplicate ID “product”.", + outcome(File.binread(vector("invalid/duplicate-id.html"))).messages.first[:message] + assert_equal "the Nu Html Checker reports 2 error(s) in the HTML view: Legacy encoding “windows-1252” used. " \ + "Documents must use UTF-8.; line 4: The only allowed value for the “charset” attribute for the “meta” " \ + "element is “utf-8”.", + outcome(File.binread(vector("invalid/legacy-encoding.html")), type: "text/html; charset=windows-1252").messages.first[:message] + end + + test "warnings and other info messages do not count" do + recorded = JSON.parse(File.read(vector("checker/valid-warnings-only.json")))["messages"] + assert_equal %w[info info], recorded.map { |m| m["type"] } + assert_empty outcome(File.binread(vector("valid/warnings-only.html"))).messages + end + + test "more than ten errors are cut short" do + errors = (1..12).map { |i| { "type" => "error", "lastLine" => i, "message" => "Error #{i}." } } + checker = server { [0, json({ "messages" => errors }.to_json)] } + message = outcome("

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)