Skip to content

Commit

Permalink
fix(webconnectivitylte): make IDNA WAI (#1460)
Browse files Browse the repository at this point in the history
## Checklist

- [x] I have read the [contribution
guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request:
ooni/probe#2652,
ooni/probe#1925
- [x] if you changed anything related to how experiments work and you
need to reflect these changes in the ooni/spec repository, please link
to the related ooni/spec pull request: not needed
- [x] if you changed code inside an experiment, make sure you bump its
version number: already bumped for 3.21.x

## Description

This diff fixes webconnectivitylte to make IDNA support working as
intended.

It turns out existing code for supporting IDNA was not WAI for yandex.ru
when using https://яндекс.рф/. I have fixed IDNA code, ensured we always
use the same correct IDNA code, manually tested (see below), and then
generated syntethic test cases for continuous integration.

See this measurement:
https://explorer.ooni.org/m/20240123110911.104106_IT_webconnectivity_f0a3b891c176125b.

While there, I started adding initial support for printing the
measurement URL based on the measurement UID.

Closes ooni/probe#1925.

Part of ooni/probe#2652.
  • Loading branch information
bassosimone committed Jan 23, 2024
1 parent a608f3a commit 924cc55
Show file tree
Hide file tree
Showing 23 changed files with 19,929 additions and 18 deletions.
58 changes: 58 additions & 0 deletions internal/experiment/webconnectivityqa/idna.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package webconnectivityqa

import (
"github.com/ooni/probe-cli/v3/internal/netemx"
)

// idnaWithoutCensorshipLowercase verifies that we can handle IDNA with lowercase.
func idnaWithoutCensorshipLowercase() *TestCase {
return &TestCase{
Name: "idnaWithoutCensorshipLowercase",
Flags: TestCaseFlagNoV04,
Input: "http://яндекс.рф/",
Configure: func(env *netemx.QAEnv) {
// nothing
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
DNSExperimentFailure: nil,
DNSConsistency: "consistent",
HTTPExperimentFailure: nil,
BodyLengthMatch: true,
BodyProportion: 1,
StatusCodeMatch: true,
HeadersMatch: true,
TitleMatch: true,
XBlockingFlags: 32, // AnalysisBlockingFlagSuccess
Accessible: true,
Blocking: false,
},
}
}

// idnaWithoutCensorshipWithFirstLetterUppercase verifies that we can handle IDNA
// with the first letter being uppercase.
func idnaWithoutCensorshipWithFirstLetterUppercase() *TestCase {
return &TestCase{
Name: "idnaWithoutCensorshipWithFirstLetterUppercase",
Flags: TestCaseFlagNoV04,
Input: "http://Яндекс.рф/",
Configure: func(env *netemx.QAEnv) {
// nothing
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
DNSExperimentFailure: nil,
DNSConsistency: "consistent",
HTTPExperimentFailure: nil,
BodyLengthMatch: true,
BodyProportion: 1,
StatusCodeMatch: true,
HeadersMatch: true,
TitleMatch: true,
XBlockingFlags: 32, // AnalysisBlockingFlagSuccess
Accessible: true,
Blocking: false,
},
}
}
3 changes: 3 additions & 0 deletions internal/experiment/webconnectivityqa/testcase.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ func AllTestCases() []*TestCase {
httpDiffWithConsistentDNS(),
httpDiffWithInconsistentDNS(),

idnaWithoutCensorshipLowercase(),
idnaWithoutCensorshipWithFirstLetterUppercase(),

redirectWithConsistentDNSAndThenConnectionRefusedForHTTP(),
redirectWithConsistentDNSAndThenConnectionRefusedForHTTPS(),
redirectWithConsistentDNSAndThenConnectionResetForHTTP(),
Expand Down
9 changes: 9 additions & 0 deletions internal/idnax/idnax.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Package idnax contains IDNA extensions.
package idnax

import "golang.org/x/net/idna"

// ToASCII converts an IDNA to ASCII using the [idna.Lookup] profile.
func ToASCII(domain string) (string, error) {
return idna.Lookup.ToASCII(domain)
}
65 changes: 65 additions & 0 deletions internal/idnax/idnax_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package idnax

import (
"errors"
"testing"

"github.com/google/go-cmp/cmp"
)

func TestToASCII(t *testing.T) {
// testcase is a test case implemented by this function.
type testcase struct {
// input is the input domain
input string

// expectErr is the expected error
expectErr error

// expectDomain is the expected domain
expectDomain string
}

testcases := []testcase{{
input: "ουτοπία.δπθ.gr",
expectErr: nil,
expectDomain: "xn--kxae4bafwg.xn--pxaix.gr",
}, {
input: "example.com",
expectErr: nil,
expectDomain: "example.com",
}, {
input: "Яндекс.рф",
expectErr: nil,
expectDomain: "xn--d1acpjx3f.xn--p1ai",
}, {
// See https://www.farsightsecurity.com/blog/txt-record/punycode-20180711/
input: "http://xn--0000h/",
expectErr: errors.New("idna: disallowed rune U+003A"),
expectDomain: "",
}}

for _, tc := range testcases {
t.Run(tc.input, func(t *testing.T) {
output, err := ToASCII(tc.input)

switch {
case err == nil && tc.expectErr == nil:
if diff := cmp.Diff(tc.expectDomain, output); diff != "" {
t.Fatal(diff)
}

case err == nil && tc.expectErr != nil:
t.Fatal("expected", tc.expectErr, "got", err)

case err != nil && tc.expectErr == nil:
t.Fatal("expected", tc.expectErr, "got", err)

case err != nil && tc.expectErr != nil:
if err.Error() != tc.expectErr.Error() {
t.Fatal("expected", tc.expectErr, "got", err)
}
}
})
}
}
4 changes: 2 additions & 2 deletions internal/inputparser/inputparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (
"net/url"
"reflect"

"github.com/ooni/probe-cli/v3/internal/idnax"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/runtimex"
"golang.org/x/net/idna"
)

// Config contains config for parsing experiments input. You MUST set
Expand Down Expand Up @@ -127,7 +127,7 @@ func maybeConvertHostnameToASCII(URL *url.URL) (*url.URL, error) {
hostname := URL.Hostname()

// Obtain an ASCII representation of the URL.Hostname().
asciiHostname, err := idna.ToASCII(hostname)
asciiHostname, err := idnax.ToASCII(hostname)
if err != nil {
return nil, fmt.Errorf("%w: %s", ErrIDNAToASCII, err.Error())
}
Expand Down
Loading

0 comments on commit 924cc55

Please sign in to comment.