Skip to content

Commit

Permalink
fix(report/handlers): accept more semver versions (#111)
Browse files Browse the repository at this point in the history
* fix(report/handlers): accept more semver versions

The current problem we have, specifically, is that since v1.2.0 the
mobile apps emit full semver including also the build.

This requires several parts of our infrastructure to improve the
regexp being used so to validate also this kind of input.

See also: measurement-kit/measurement-kit#1388

An earlier version of this diff was blessed by @hellais on Slack and
since then I just changed comments.

* attempt to repair regress test

* fix problem with keyboard and chair

* another approach

* improve over previous

* also print pid after the event

* make sure it is killed

* Finish off PR

* rewrite the test

* exchange the order of operations to avoid a race
  • Loading branch information
bassosimone committed Sep 26, 2017
1 parent 03d738b commit 8ccede1
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 44 deletions.
35 changes: 0 additions & 35 deletions .travis.test.sh

This file was deleted.

29 changes: 26 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ install:
- pip install -r requirements.txt
- pip install coveralls
- python setup.py install
# command to run tests, e.g. python setup.py test

script:

# 1. prepare for running oonib
- cp oonib.conf.example oonib.conf
- openssl genrsa -des3 -passout pass:oonib -out private.key 4096
- openssl req -batch -passin pass:oonib -new -key private.key -out server.csr
Expand All @@ -37,8 +39,29 @@ script:
- openssl x509 -req -days 365 -in server.csr -signkey private.key -out certificate.crt
- test -f "private.key.org" && rm -f private.key.org
- test -f "server.csr" && rm -f server.csr
- chmod +x .travis.test.sh && ./.travis.test.sh 30 ./bin/oonib
- echo "Build successful."

# 2. smoke test oonib: does it startup, does it react to SIGTERM?
- ./bin/oonib &
# Notes on smoke testing
# ----------------------
#
# 1. The original smoke testing procedure was much more robust but travis
# started segfaulting and, upon investigating that, it occurred to me
# that having a simplified procedure like the one below would also work.
#
# Specifically, if the process does not exit after `TERM`, my understanding
# is that the build will hang until travis kills it. This would fail the
# build and so we would know that the deamon doesn't handle `TERM`.
#
# 2. I need to copy the pidfile because it is deleted by the daemon when
# it exits, as such, using the original pidfile is racy.
- sleep 30
- cp oonib.pid oonib-persist.pid
- kill -TERM `cat oonib-persist.pid`
- wait `cat oonib-persist.pid`

# 3. run regress tests
- $(which coverage) run $(which trial) oonib

after_success:
- coveralls --rcfile=".coveragerc"
13 changes: 12 additions & 1 deletion oonib/report/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,18 @@ def parseUpdateReportRequest(request, report_id=None):


def validateHeader(header):
version_string = re.compile(r"^[0-9A-Za-z_\-\.]+$")
# Version-number-regex context
# ----------------------------
#
# We are updating our software to always use semver, but for historical
# reasons there are also cases where we use '0.1', so better to be
# very liberal an accept anything version-ish in here.
#
# Live regexp testing: https://regex101.com/r/zhnfFl/3
#
# See also: github.com/measurement-kit/measurement-kit/pull/1388
version_string = re.compile(r"^[0-9A-Za-z_.+-]+$")

name = re.compile(r"^[a-zA-Z0-9_\- ]+$")
probe_asn = re.compile(r"^AS[0-9]+$")
probe_cc = re.compile(r"^[A-Z]{2}$")
Expand Down
23 changes: 18 additions & 5 deletions oonib/tests/test_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@
from oonib.report.api import reportAPI
from oonib.tests.handler_helpers import HandlerTestCase, mock_initialize, MockTime

# Version number testing
# ----------------------
#
# We need to test an array of version numbers ranging from super compliant
# with semver (mobile apps since v1.2.0) to very basic (e.g. 0.1, as used
# in some cases by ooni-probe test implementations).
#
# To emphasize this, I have refactored this regress test to put the
# various version numbers we use here at the beginning of file.
FULL_SEMVER_VERSION = "1.7.0-beta.1+11.7"
BASIC_SEMVER_VERSION = "1.7.0"
SIMPLE_VERSION = "1.0"

sample_report_entry = {
'agent': 'agent',
'input': 'http://example.com',
Expand Down Expand Up @@ -42,7 +55,7 @@
'probe_city': None,
'probe_ip': '127.0.0.1',
'software_name': 'ooniprobe',
'software_version': '1.1.0',
'software_version': FULL_SEMVER_VERSION,
'test_start_time': '2016-01-01 12:34:56',
'test_name': 'fake_test',
'test_version': '0.1.0'
Expand All @@ -60,18 +73,18 @@
probe_city: null
probe_ip: 127.0.0.1
software_name: ooniprobe
software_version: 1.1.0
software_version: %s
test_start_time: '2016-01-01 12:34:56'
test_name: fake_test
test_version: 0.1.0
...
"""
""" % BASIC_SEMVER_VERSION

dummy_data = {
'software_name': 'ooni-test',
'software_version': '0.1',
'software_version': SIMPLE_VERSION,
'test_name': 'some-test',
'test_version': '0.1',
'test_version': SIMPLE_VERSION,
'probe_asn': 'AS0',
'probe_cc': 'ZZ',
'test_start_time': '2016-01-01 12:34:56'
Expand Down

0 comments on commit 8ccede1

Please sign in to comment.