diff --git a/.circleci/Dockerfile.debian b/.circleci/Dockerfile.debian index f12f19551..abab1f4fa 100644 --- a/.circleci/Dockerfile.debian +++ b/.circleci/Dockerfile.debian @@ -18,15 +18,11 @@ RUN apt-get --quiet update && \ libffi-dev \ libssl-dev \ libyaml-dev \ - virtualenv + virtualenv \ + tor # Get the project source. This is better than it seems. CircleCI will # *update* this checkout on each job run, saving us more time per-job. COPY . ${BUILD_SRC_ROOT} RUN "${BUILD_SRC_ROOT}"/.circleci/prepare-image.sh "${WHEELHOUSE_PATH}" "${VIRTUALENV_PATH}" "${BUILD_SRC_ROOT}" "python${PYTHON_VERSION}" - -# Only the integration tests currently need this but it doesn't hurt to always -# have it present and it's simpler than building a whole extra image just for -# the integration tests. -RUN ${BUILD_SRC_ROOT}/integration/install-tor.sh diff --git a/.circleci/config.yml b/.circleci/config.yml index 79ce57ed0..051e690b7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -18,8 +18,7 @@ workflows: - "debian-10": {} - "debian-11": - requires: - - "debian-10" + {} - "ubuntu-20-04": {} @@ -58,7 +57,7 @@ workflows: requires: # If the unit test suite doesn't pass, don't bother running the # integration tests. - - "debian-10" + - "debian-11" - "typechecks": {} @@ -297,6 +296,10 @@ jobs: integration: <<: *DEBIAN + docker: + - <<: *DOCKERHUB_AUTH + image: "tahoelafsci/debian:11-py3.9" + user: "nobody" environment: <<: *UTF_8_ENVIRONMENT diff --git a/.circleci/run-tests.sh b/.circleci/run-tests.sh index 764651c40..854013c32 100755 --- a/.circleci/run-tests.sh +++ b/.circleci/run-tests.sh @@ -52,7 +52,7 @@ fi # This is primarily aimed at catching hangs on the PyPy job which runs for # about 21 minutes and then gets killed by CircleCI in a way that fails the # job and bypasses our "allowed failure" logic. -TIMEOUT="timeout --kill-after 1m 15m" +TIMEOUT="timeout --kill-after 1m 25m" # Run the test suite as a non-root user. This is the expected usage some # small areas of the test suite assume non-root privileges (such as unreadable diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0327014ca..99ac28926 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -163,7 +163,9 @@ jobs: matrix: os: - windows-latest - - ubuntu-latest + # 22.04 has some issue with Tor at the moment: + # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3943 + - ubuntu-20.04 python-version: - 3.7 - 3.9 @@ -175,7 +177,7 @@ jobs: steps: - name: Install Tor [Ubuntu] - if: matrix.os == 'ubuntu-latest' + if: ${{ contains(matrix.os, 'ubuntu') }} run: sudo apt install tor # TODO: See https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3744. diff --git a/Makefile b/Makefile index 5cbd863a3..c02184a36 100644 --- a/Makefile +++ b/Makefile @@ -224,3 +224,62 @@ src/allmydata/_version.py: .tox/create-venvs.log: tox.ini setup.py tox --notest -p all | tee -a "$(@)" + + +# to make a new release: +# - create a ticket for the release in Trac +# - ensure local copy is up-to-date +# - create a branch like "XXXX.release" from up-to-date master +# - in the branch, run "make release" +# - run "make release-test" +# - perform any other sanity-checks on the release +# - run "make release-upload" +# Note that several commands below hard-code "meejah"; if you are +# someone else please adjust them. +release: + @echo "Is checkout clean?" + git diff-files --quiet + git diff-index --quiet --cached HEAD -- + + @echo "Clean docs build area" + rm -rf docs/_build/ + + @echo "Install required build software" + python3 -m pip install --editable .[build] + + @echo "Test README" + python3 setup.py check -r -s + + @echo "Update NEWS" + python3 -m towncrier build --yes --version `python3 misc/build_helpers/update-version.py --no-tag` + git add -u + git commit -m "update NEWS for release" + +# note that this always bumps the "middle" number, e.g. from 1.17.1 -> 1.18.0 +# and produces a tag into the Git repository + @echo "Bump version and create tag" + python3 misc/build_helpers/update-version.py + + @echo "Build and sign wheel" + python3 setup.py bdist_wheel + gpg --pinentry=loopback -u meejah@meejah.ca --armor --detach-sign dist/tahoe_lafs-`git describe | cut -b 12-`-py3-none-any.whl + ls dist/*`git describe | cut -b 12-`* + + @echo "Build and sign source-dist" + python3 setup.py sdist + gpg --pinentry=loopback -u meejah@meejah.ca --armor --detach-sign dist/tahoe-lafs-`git describe | cut -b 12-`.tar.gz + ls dist/*`git describe | cut -b 12-`* + +# basically just a bare-minimum smoke-test that it installs and runs +release-test: + gpg --verify dist/tahoe-lafs-`git describe | cut -b 12-`.tar.gz.asc + gpg --verify dist/tahoe_lafs-`git describe | cut -b 12-`-py3-none-any.whl.asc + virtualenv testmf_venv + testmf_venv/bin/pip install dist/tahoe_lafs-`git describe | cut -b 12-`-py3-none-any.whl + testmf_venv/bin/tahoe --version + rm -rf testmf_venv + +release-upload: + scp dist/*`git describe | cut -b 12-`* meejah@tahoe-lafs.org:/home/source/downloads + git push origin_push tahoe-lafs-`git describe | cut -b 12-` + twine upload dist/tahoe_lafs-`git describe | cut -b 12-`-py3-none-any.whl dist/tahoe_lafs-`git describe | cut -b 12-`-py3-none-any.whl.asc dist/tahoe-lafs-`git describe | cut -b 12-`.tar.gz dist/tahoe-lafs-`git describe | cut -b 12-`.tar.gz.asc diff --git a/NEWS.rst b/NEWS.rst index 0f9194cc4..7b1fadb8a 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -5,6 +5,47 @@ User-Visible Changes in Tahoe-LAFS ================================== .. towncrier start line +Release 1.18.0 (2022-10-02) +''''''''''''''''''''''''''' + +Backwards Incompatible Changes +------------------------------ + +- Python 3.6 is no longer supported, as it has reached end-of-life and is no longer receiving security updates. (`#3865 `_) +- Python 3.7 or later is now required; Python 2 is no longer supported. (`#3873 `_) +- Share corruption reports stored on disk are now always encoded in UTF-8. (`#3879 `_) +- Record both the PID and the process creation-time: + + a new kind of pidfile in `running.process` records both + the PID and the creation-time of the process. This facilitates + automatic discovery of a "stale" pidfile that points to a + currently-running process. If the recorded creation-time matches + the creation-time of the running process, then it is a still-running + `tahoe run` process. Otherwise, the file is stale. + + The `twistd.pid` file is no longer present. (`#3926 `_) + + +Features +-------- + +- The implementation of SDMF and MDMF (mutables) now requires RSA keys to be exactly 2048 bits, aligning them with the specification. + + Some code existed to allow tests to shorten this and it's + conceptually possible a modified client produced mutables + with different key-sizes. However, the spec says that they + must be 2048 bits. If you happen to have a capability with + a key-size different from 2048 you may use 1.17.1 or earlier + to read the content. (`#3828 `_) +- "make" based release automation (`#3846 `_) + + +Misc/Other +---------- + +- `#3327 `_, `#3526 `_, `#3697 `_, `#3709 `_, `#3786 `_, `#3788 `_, `#3802 `_, `#3816 `_, `#3855 `_, `#3858 `_, `#3859 `_, `#3860 `_, `#3867 `_, `#3868 `_, `#3871 `_, `#3872 `_, `#3875 `_, `#3876 `_, `#3877 `_, `#3881 `_, `#3882 `_, `#3883 `_, `#3889 `_, `#3890 `_, `#3891 `_, `#3893 `_, `#3895 `_, `#3896 `_, `#3898 `_, `#3900 `_, `#3909 `_, `#3913 `_, `#3915 `_, `#3916 `_ + + Release 1.17.1 (2022-01-07) ''''''''''''''''''''''''''' diff --git a/docs/check_running.py b/docs/check_running.py new file mode 100644 index 000000000..2705f1721 --- /dev/null +++ b/docs/check_running.py @@ -0,0 +1,47 @@ + +import psutil +import filelock + + +def can_spawn_tahoe(pidfile): + """ + Determine if we can spawn a Tahoe-LAFS for the given pidfile. That + pidfile may be deleted if it is stale. + + :param pathlib.Path pidfile: the file to check, that is the Path + to "running.process" in a Tahoe-LAFS configuration directory + + :returns bool: True if we can spawn `tahoe run` here + """ + lockpath = pidfile.parent / (pidfile.name + ".lock") + with filelock.FileLock(lockpath): + try: + with pidfile.open("r") as f: + pid, create_time = f.read().strip().split(" ", 1) + except FileNotFoundError: + return True + + # somewhat interesting: we have a pidfile + pid = int(pid) + create_time = float(create_time) + + try: + proc = psutil.Process(pid) + # most interesting case: there _is_ a process running at the + # recorded PID -- but did it just happen to get that PID, or + # is it the very same one that wrote the file? + if create_time == proc.create_time(): + # _not_ stale! another intance is still running against + # this configuration + return False + + except psutil.NoSuchProcess: + pass + + # the file is stale + pidfile.unlink() + return True + + +from pathlib import Path +print("can spawn?", can_spawn_tahoe(Path("running.process"))) diff --git a/docs/conf.py b/docs/conf.py index af05e5900..cc9a11166 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -63,7 +63,7 @@ release = u'1.x' # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = None +language = "en" # There are two options for replacing |today|: either, you set today to some # non-false value, then it is used: diff --git a/docs/proposed/http-storage-node-protocol.rst b/docs/proposed/http-storage-node-protocol.rst index 3926d9f4a..aee201cf5 100644 --- a/docs/proposed/http-storage-node-protocol.rst +++ b/docs/proposed/http-storage-node-protocol.rst @@ -30,12 +30,12 @@ Glossary introducer a Tahoe-LAFS process at a known location configured to re-publish announcements about the location of storage servers - fURL + :ref:`fURLs ` a self-authenticating URL-like string which can be used to locate a remote object using the Foolscap protocol (the storage service is an example of such an object) - NURL - a self-authenticating URL-like string almost exactly like a NURL but without being tied to Foolscap + :ref:`NURLs ` + a self-authenticating URL-like string almost exactly like a fURL but without being tied to Foolscap swissnum a short random string which is part of a fURL/NURL and which acts as a shared secret to authorize clients to use a storage service @@ -350,8 +350,10 @@ Because of the simple types used throughout and the equivalence described in `RFC 7049`_ these examples should be representative regardless of which of these two encodings is chosen. +The one exception is sets. For CBOR messages, any sequence that is semantically a set (i.e. no repeated values allowed, order doesn't matter, and elements are hashable in Python) should be sent as a set. Tag 6.258 is used to indicate sets in CBOR; see `the CBOR registry `_ for more details. +Sets will be represented as JSON lists in examples because JSON doesn't support sets. HTTP Design ~~~~~~~~~~~ @@ -393,8 +395,8 @@ Encoding General ~~~~~~~ -``GET /v1/version`` -!!!!!!!!!!!!!!!!!!! +``GET /storage/v1/version`` +!!!!!!!!!!!!!!!!!!!!!!!!!!! Retrieve information about the version of the storage server. Information is returned as an encoded mapping. @@ -407,14 +409,13 @@ For example:: "tolerates-immutable-read-overrun": true, "delete-mutable-shares-with-zero-length-writev": true, "fills-holes-with-zero-bytes": true, - "prevents-read-past-end-of-share-data": true, - "gbs-anonymous-storage-url": "pb://...#v=1" + "prevents-read-past-end-of-share-data": true }, "application-version": "1.13.0" } -``PUT /v1/lease/:storage_index`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +``PUT /storage/v1/lease/:storage_index`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Either renew or create a new lease on the bucket addressed by ``storage_index``. @@ -466,8 +467,8 @@ Immutable Writing ~~~~~~~ -``POST /v1/immutable/:storage_index`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +``POST /storage/v1/immutable/:storage_index`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Initialize an immutable storage index with some buckets. The buckets may have share data written to them once. @@ -502,7 +503,7 @@ Handling repeat calls: Discussion `````````` -We considered making this ``POST /v1/immutable`` instead. +We considered making this ``POST /storage/v1/immutable`` instead. The motivation was to keep *storage index* out of the request URL. Request URLs have an elevated chance of being logged by something. We were concerned that having the *storage index* logged may increase some risks. @@ -537,8 +538,8 @@ Rejected designs for upload secrets: it must contain randomness. Randomness means there is no need to have a secret per share, since adding share-specific content to randomness doesn't actually make the secret any better. -``PATCH /v1/immutable/:storage_index/:share_number`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +``PATCH /storage/v1/immutable/:storage_index/:share_number`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Write data for the indicated share. The share number must belong to the storage index. @@ -578,24 +579,6 @@ Responses: the response is ``CONFLICT``. At this point the only thing to do is abort the upload and start from scratch (see below). -``PUT /v1/immutable/:storage_index/:share_number/abort`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - -This cancels an *in-progress* upload. - -The request must include a ``X-Tahoe-Authorization`` header that includes the upload secret:: - - X-Tahoe-Authorization: upload-secret - -The response code: - -* When the upload is still in progress and therefore the abort has succeeded, - the response is ``OK``. - Future uploads can start from scratch with no pre-existing upload state stored on the server. -* If the uploaded has already finished, the response is 405 (Method Not Allowed) - and no change is made. - - Discussion `````````` @@ -614,8 +597,27 @@ From RFC 7231:: PATCH method defined in [RFC5789]). -``POST /v1/immutable/:storage_index/:share_number/corrupt`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + +``PUT /storage/v1/immutable/:storage_index/:share_number/abort`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + +This cancels an *in-progress* upload. + +The request must include a ``X-Tahoe-Authorization`` header that includes the upload secret:: + + X-Tahoe-Authorization: upload-secret + +The response code: + +* When the upload is still in progress and therefore the abort has succeeded, + the response is ``OK``. + Future uploads can start from scratch with no pre-existing upload state stored on the server. +* If the uploaded has already finished, the response is 405 (Method Not Allowed) + and no change is made. + + +``POST /storage/v1/immutable/:storage_index/:share_number/corrupt`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Advise the server the data read from the indicated share was corrupt. The request body includes an human-meaningful text string with details about the @@ -623,7 +625,7 @@ corruption. It also includes potentially important details about the share. For example:: - {"reason": u"expected hash abcd, got hash efgh"} + {"reason": "expected hash abcd, got hash efgh"} .. share-type, storage-index, and share-number are inferred from the URL @@ -633,8 +635,8 @@ couldn't be found. Reading ~~~~~~~ -``GET /v1/immutable/:storage_index/shares`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +``GET /storage/v1/immutable/:storage_index/shares`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Retrieve a list (semantically, a set) indicating all shares available for the indicated storage index. For example:: @@ -643,8 +645,8 @@ indicated storage index. For example:: An unknown storage index results in an empty list. -``GET /v1/immutable/:storage_index/:share_number`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +``GET /storage/v1/immutable/:storage_index/:share_number`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Read a contiguous sequence of bytes from one share in one bucket. The response body is the raw share data (i.e., ``application/octet-stream``). @@ -652,6 +654,11 @@ The ``Range`` header may be used to request exactly one ``bytes`` range, in whic Interpretation and response behavior is as specified in RFC 7233 § 4.1. Multiple ranges in a single request are *not* supported; open-ended ranges are also not supported. +If the response reads beyond the end of the data, the response may be shorter than the requested range. +The resulting ``Content-Range`` header will be consistent with the returned data. + +If the response to a query is an empty range, the ``NO CONTENT`` (204) response code will be used. + Discussion `````````` @@ -679,8 +686,8 @@ Mutable Writing ~~~~~~~ -``POST /v1/mutable/:storage_index/read-test-write`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +``POST /storage/v1/mutable/:storage_index/read-test-write`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! General purpose read-test-and-write operation for mutable storage indexes. A mutable storage index is also called a "slot" @@ -735,26 +742,31 @@ As a result, if there is no data at all, an empty bytestring is returned no matt Reading ~~~~~~~ -``GET /v1/mutable/:storage_index/shares`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +``GET /storage/v1/mutable/:storage_index/shares`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -Retrieve a list indicating all shares available for the indicated storage index. -For example:: +Retrieve a set indicating all shares available for the indicated storage index. +For example (this is shown as list, since it will be list for JSON, but will be set for CBOR):: [1, 5] -``GET /v1/mutable/:storage_index/:share_number`` +``GET /storage/v1/mutable/:storage_index/:share_number`` !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -Read data from the indicated mutable shares, just like ``GET /v1/immutable/:storage_index`` +Read data from the indicated mutable shares, just like ``GET /storage/v1/immutable/:storage_index`` The ``Range`` header may be used to request exactly one ``bytes`` range, in which case the response code will be 206 (partial content). Interpretation and response behavior is as specified in RFC 7233 § 4.1. Multiple ranges in a single request are *not* supported; open-ended ranges are also not supported. +If the response reads beyond the end of the data, the response may be shorter than the requested range. +The resulting ``Content-Range`` header will be consistent with the returned data. -``POST /v1/mutable/:storage_index/:share_number/corrupt`` -!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! +If the response to a query is an empty range, the ``NO CONTENT`` (204) response code will be used. + + +``POST /storage/v1/mutable/:storage_index/:share_number/corrupt`` +!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Advise the server the data read from the indicated share was corrupt. Just like the immutable version. @@ -767,7 +779,7 @@ Immutable Data 1. Create a bucket for storage index ``AAAAAAAAAAAAAAAA`` to hold two immutable shares, discovering that share ``1`` was already uploaded:: - POST /v1/immutable/AAAAAAAAAAAAAAAA + POST /storage/v1/immutable/AAAAAAAAAAAAAAAA Authorization: Tahoe-LAFS nurl-swissnum X-Tahoe-Authorization: lease-renew-secret efgh X-Tahoe-Authorization: lease-cancel-secret jjkl @@ -780,23 +792,25 @@ Immutable Data #. Upload the content for immutable share ``7``:: - PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7 + PATCH /storage/v1/immutable/AAAAAAAAAAAAAAAA/7 Authorization: Tahoe-LAFS nurl-swissnum Content-Range: bytes 0-15/48 X-Tahoe-Authorization: upload-secret xyzf 200 OK + { "required": [ {"begin": 16, "end": 48 } ] } - PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7 + PATCH /storage/v1/immutable/AAAAAAAAAAAAAAAA/7 Authorization: Tahoe-LAFS nurl-swissnum Content-Range: bytes 16-31/48 X-Tahoe-Authorization: upload-secret xyzf 200 OK + { "required": [ {"begin": 32, "end": 48 } ] } - PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7 + PATCH /storage/v1/immutable/AAAAAAAAAAAAAAAA/7 Authorization: Tahoe-LAFS nurl-swissnum Content-Range: bytes 32-47/48 X-Tahoe-Authorization: upload-secret xyzf @@ -806,16 +820,17 @@ Immutable Data #. Download the content of the previously uploaded immutable share ``7``:: - GET /v1/immutable/AAAAAAAAAAAAAAAA?share=7 + GET /storage/v1/immutable/AAAAAAAAAAAAAAAA?share=7 Authorization: Tahoe-LAFS nurl-swissnum Range: bytes=0-47 200 OK + Content-Range: bytes 0-47/48 #. Renew the lease on all immutable shares in bucket ``AAAAAAAAAAAAAAAA``:: - PUT /v1/lease/AAAAAAAAAAAAAAAA + PUT /storage/v1/lease/AAAAAAAAAAAAAAAA Authorization: Tahoe-LAFS nurl-swissnum X-Tahoe-Authorization: lease-cancel-secret jjkl X-Tahoe-Authorization: lease-renew-secret efgh @@ -830,7 +845,7 @@ The special test vector of size 1 but empty bytes will only pass if there is no existing share, otherwise it will read a byte which won't match `b""`:: - POST /v1/mutable/BBBBBBBBBBBBBBBB/read-test-write + POST /storage/v1/mutable/BBBBBBBBBBBBBBBB/read-test-write Authorization: Tahoe-LAFS nurl-swissnum X-Tahoe-Authorization: write-enabler abcd X-Tahoe-Authorization: lease-cancel-secret efgh @@ -862,7 +877,7 @@ otherwise it will read a byte which won't match `b""`:: #. Safely rewrite the contents of a known version of mutable share number ``3`` (or fail):: - POST /v1/mutable/BBBBBBBBBBBBBBBB/read-test-write + POST /storage/v1/mutable/BBBBBBBBBBBBBBBB/read-test-write Authorization: Tahoe-LAFS nurl-swissnum X-Tahoe-Authorization: write-enabler abcd X-Tahoe-Authorization: lease-cancel-secret efgh @@ -894,14 +909,17 @@ otherwise it will read a byte which won't match `b""`:: #. Download the contents of share number ``3``:: - GET /v1/mutable/BBBBBBBBBBBBBBBB?share=3&offset=0&size=10 + GET /storage/v1/mutable/BBBBBBBBBBBBBBBB?share=3 Authorization: Tahoe-LAFS nurl-swissnum + Range: bytes=0-16 + 200 OK + Content-Range: bytes 0-15/16 #. Renew the lease on previously uploaded mutable share in slot ``BBBBBBBBBBBBBBBB``:: - PUT /v1/lease/BBBBBBBBBBBBBBBB + PUT /storage/v1/lease/BBBBBBBBBBBBBBBB Authorization: Tahoe-LAFS nurl-swissnum X-Tahoe-Authorization: lease-cancel-secret efgh X-Tahoe-Authorization: lease-renew-secret ijkl diff --git a/docs/running.rst b/docs/running.rst index 406c8200b..263448735 100644 --- a/docs/running.rst +++ b/docs/running.rst @@ -124,6 +124,35 @@ Tahoe-LAFS. .. _magic wormhole: https://magic-wormhole.io/ +Multiple Instances +------------------ + +Running multiple instances against the same configuration directory isn't supported. +This will lead to undefined behavior and could corrupt the configuration or state. + +We attempt to avoid this situation with a "pidfile"-style file in the config directory called ``running.process``. +There may be a parallel file called ``running.process.lock`` in existence. + +The ``.lock`` file exists to make sure only one process modifies ``running.process`` at once. +The lock file is managed by the `lockfile `_ library. +If you wish to make use of ``running.process`` for any reason you should also lock it and follow the semantics of lockfile. + +If ``running.process`` exists then it contains the PID and the creation-time of the process. +When no such file exists, there is no other process running on this configuration. +If there is a ``running.process`` file, it may be a leftover file or it may indicate that another process is running against this config. +To tell the difference, determine if the PID in the file exists currently. +If it does, check the creation-time of the process versus the one in the file. +If these match, there is another process currently running and using this config. +Otherwise, the file is stale -- it should be removed before starting Tahoe-LAFS. + +Some example Python code to check the above situations: + +.. literalinclude:: check_running.py + + + + + A note about small grids ------------------------ diff --git a/docs/specifications/url.rst b/docs/specifications/url.rst index 31fb05fad..12e2b8642 100644 --- a/docs/specifications/url.rst +++ b/docs/specifications/url.rst @@ -7,6 +7,8 @@ These are not to be confused with the URI-like capabilities Tahoe-LAFS uses to r An attempt is also made to outline the rationale for certain choices about these URLs. The intended audience for this document is Tahoe-LAFS maintainers and other developers interested in interoperating with Tahoe-LAFS or these URLs. +.. _furls: + Background ---------- @@ -31,6 +33,8 @@ The client's use of the swissnum is what allows the server to authorize the clie .. _`swiss number`: http://wiki.erights.org/wiki/Swiss_number +.. _NURLs: + NURLs ----- @@ -47,27 +51,27 @@ This can be considered to expand to "**N**\ ew URLs" or "Authe\ **N**\ ticating The anticipated use for a **NURL** will still be to establish a TLS connection to a peer. The protocol run over that TLS connection could be Foolscap though it is more likely to be an HTTP-based protocol (such as GBS). +Unlike fURLs, only a single net-loc is included, for consistency with other forms of URLs. +As a result, multiple NURLs may be available for a single server. + Syntax ------ The EBNF for a NURL is as follows:: - nurl = scheme, hash, "@", net-loc-list, "/", swiss-number, [ version1 ] - - scheme = "pb://" + nurl = tcp-nurl | tor-nurl | i2p-nurl + tcp-nurl = "pb://", hash, "@", tcp-loc, "/", swiss-number, [ version1 ] + tor-nurl = "pb+tor://", hash, "@", tcp-loc, "/", swiss-number, [ version1 ] + i2p-nurl = "pb+i2p://", hash, "@", i2p-loc, "/", swiss-number, [ version1 ] hash = unreserved - net-loc-list = net-loc, [ { ",", net-loc } ] - net-loc = tcp-loc | tor-loc | i2p-loc - - tcp-loc = [ "tcp:" ], hostname, [ ":" port ] - tor-loc = "tor:", hostname, [ ":" port ] - i2p-loc = "i2p:", i2p-addr, [ ":" port ] - - i2p-addr = { unreserved }, ".i2p" + tcp-loc = hostname, [ ":" port ] hostname = domain | IPv4address | IPv6address + i2p-loc = i2p-addr, [ ":" port ] + i2p-addr = { unreserved }, ".i2p" + swiss-number = segment version1 = "#v=1" @@ -87,11 +91,13 @@ These differences are separated into distinct versions. Version 0 --------- -A Foolscap fURL is considered the canonical definition of a version 0 NURL. +In theory, a Foolscap fURL with a single netloc is considered the canonical definition of a version 0 NURL. Notably, the hash component is defined as the base32-encoded SHA1 hash of the DER form of an x509v3 certificate. A version 0 NURL is identified by the absence of the ``v=1`` fragment. +In practice, real world fURLs may have more than one netloc, so lack of version fragment will likely just involve dispatching the fURL to a different parser. + Examples ~~~~~~~~ @@ -103,11 +109,8 @@ Version 1 The hash component of a version 1 NURL differs in three ways from the prior version. -1. The hash function used is SHA3-224 instead of SHA1. - The security of SHA1 `continues to be eroded`_. - Contrariwise SHA3 is currently the most recent addition to the SHA family by NIST. - The 224 bit instance is chosen to keep the output short and because it offers greater collision resistance than SHA1 was thought to offer even at its inception - (prior to security research showing actual collision resistance is lower). +1. The hash function used is SHA-256, to match RFC 7469. + The security of SHA1 `continues to be eroded`_; Latacora `SHA-2`_. 2. The hash is computed over the certificate's SPKI instead of the whole certificate. This allows certificate re-generation so long as the public key remains the same. This is useful to allow contact information to be updated or extension of validity period. @@ -122,7 +125,7 @@ The hash component of a version 1 NURL differs in three ways from the prior vers *all* certificate fields should be considered within the context of the relationship identified by the SPKI hash. 3. The hash is encoded using urlsafe-base64 (without padding) instead of base32. - This provides a more compact representation and minimizes the usability impacts of switching from a 160 bit hash to a 224 bit hash. + This provides a more compact representation and minimizes the usability impacts of switching from a 160 bit hash to a 256 bit hash. A version 1 NURL is identified by the presence of the ``v=1`` fragment. Though the length of the hash string (38 bytes) could also be used to differentiate it from a version 0 NURL, @@ -140,7 +143,8 @@ Examples * ``pb://azEu8vlRpnEeYm0DySQDeNY3Z2iJXHC_bsbaAw@localhost:47877/64i4aokv4ej#v=1`` .. _`continues to be eroded`: https://en.wikipedia.org/wiki/SHA-1#Cryptanalysis_and_validation -.. _`explored by the web community`: https://www.imperialviolet.org/2011/05/04/pinning.html +.. _`SHA-2`: https://latacora.micro.blog/2018/04/03/cryptographic-right-answers.html +.. _`explored by the web community`: https://www.rfc-editor.org/rfc/rfc7469 .. _Foolscap: https://github.com/warner/foolscap .. [1] ``foolscap.furl.decode_furl`` is taken as the canonical definition of the syntax of a fURL. diff --git a/docs/stats.rst b/docs/stats.rst index 50642d816..c7d69e0d2 100644 --- a/docs/stats.rst +++ b/docs/stats.rst @@ -264,3 +264,18 @@ the "tahoe-conf" file for notes about configuration and installing these plugins into a Munin environment. .. _Munin: http://munin-monitoring.org/ + + +Scraping Stats Values in OpenMetrics Format +=========================================== + +Time Series DataBase (TSDB) software like Prometheus_ and VictoriaMetrics_ can +parse statistics from the e.g. http://localhost:3456/statistics?t=openmetrics +URL in OpenMetrics_ format. Software like Grafana_ can then be used to graph +and alert on these numbers. You can find a pre-configured dashboard for +Grafana at https://grafana.com/grafana/dashboards/16894-tahoe-lafs/. + +.. _OpenMetrics: https://openmetrics.io/ +.. _Prometheus: https://prometheus.io/ +.. _VictoriaMetrics: https://victoriametrics.com/ +.. _Grafana: https://grafana.com/ diff --git a/integration/install-tor.sh b/integration/install-tor.sh deleted file mode 100755 index 66fa64cb1..000000000 --- a/integration/install-tor.sh +++ /dev/null @@ -1,794 +0,0 @@ -#!/bin/bash - -# https://vaneyckt.io/posts/safer_bash_scripts_with_set_euxo_pipefail/ -set -euxo pipefail - -CODENAME=$(lsb_release --short --codename) - -if [ "$(id -u)" != "0" ]; then - SUDO="sudo" -else - SUDO="" -fi - -# Script to install Tor -echo "deb http://deb.torproject.org/torproject.org ${CODENAME} main" | ${SUDO} tee -a /etc/apt/sources.list -echo "deb-src http://deb.torproject.org/torproject.org ${CODENAME} main" | ${SUDO} tee -a /etc/apt/sources.list - -# # Install Tor repo signing key -${SUDO} apt-key add - </statistics/?t=json/` - - there is at least one storage-server connected + - there is at least one storage-server connected (configurable via + ``minimum_number_of_servers``) - every storage-server has a "last_received_data" and it is within the last `liveness` seconds @@ -506,8 +507,8 @@ def await_client_ready(tahoe, timeout=10, liveness=60*2): time.sleep(1) continue - if len(js['servers']) == 0: - print("waiting because no servers at all") + if len(js['servers']) < minimum_number_of_servers: + print("waiting because insufficient servers") time.sleep(1) continue server_times = [ diff --git a/misc/build_helpers/update-version.py b/misc/build_helpers/update-version.py new file mode 100644 index 000000000..75b22edae --- /dev/null +++ b/misc/build_helpers/update-version.py @@ -0,0 +1,95 @@ +# +# this updates the (tagged) version of the software +# +# Any "options" are hard-coded in here (e.g. the GnuPG key to use) +# + +author = "meejah " + + +import sys +import time +from datetime import datetime +from packaging.version import Version + +from dulwich.repo import Repo +from dulwich.porcelain import ( + tag_list, + tag_create, + status, +) + +from twisted.internet.task import ( + react, +) +from twisted.internet.defer import ( + ensureDeferred, +) + + +def existing_tags(git): + versions = sorted( + Version(v.decode("utf8").lstrip("tahoe-lafs-")) + for v in tag_list(git) + if v.startswith(b"tahoe-lafs-") + ) + return versions + + +def create_new_version(git): + versions = existing_tags(git) + biggest = versions[-1] + + return Version( + "{}.{}.{}".format( + biggest.major, + biggest.minor + 1, + 0, + ) + ) + + +async def main(reactor): + git = Repo(".") + + st = status(git) + if any(st.staged.values()) or st.unstaged: + print("unclean checkout; aborting") + raise SystemExit(1) + + v = create_new_version(git) + if "--no-tag" in sys.argv: + print(v) + return + + print("Existing tags: {}".format("\n".join(str(x) for x in existing_tags(git)))) + print("New tag will be {}".format(v)) + + # the "tag time" is seconds from the epoch .. we quantize these to + # the start of the day in question, in UTC. + now = datetime.now() + s = now.utctimetuple() + ts = int( + time.mktime( + time.struct_time((s.tm_year, s.tm_mon, s.tm_mday, 0, 0, 0, 0, s.tm_yday, 0)) + ) + ) + tag_create( + repo=git, + tag="tahoe-lafs-{}".format(str(v)).encode("utf8"), + author=author.encode("utf8"), + message="Release {}".format(v).encode("utf8"), + annotated=True, + objectish=b"HEAD", + sign=author.encode("utf8"), + tag_time=ts, + tag_timezone=0, + ) + + print("Tag created locally, it is not pushed") + print("To push it run something like:") + print(" git push origin {}".format(v)) + + +if __name__ == "__main__": + react(lambda r: ensureDeferred(main(r))) diff --git a/newsfragments/3526.minor b/newsfragments/3526.minor deleted file mode 100644 index 8b1378917..000000000 --- a/newsfragments/3526.minor +++ /dev/null @@ -1 +0,0 @@ - diff --git a/newsfragments/3697.minor b/newsfragments/3697.minor deleted file mode 100644 index 0977d8a6f..000000000 --- a/newsfragments/3697.minor +++ /dev/null @@ -1 +0,0 @@ -Added support for Python 3.10. Added support for PyPy3 (3.7 and 3.8, on Linux only). \ No newline at end of file diff --git a/newsfragments/3327.minor b/newsfragments/3783.minor similarity index 100% rename from newsfragments/3327.minor rename to newsfragments/3783.minor diff --git a/newsfragments/3828.feature b/newsfragments/3828.feature deleted file mode 100644 index d396439b0..000000000 --- a/newsfragments/3828.feature +++ /dev/null @@ -1,8 +0,0 @@ -The implementation of SDMF and MDMF (mutables) now requires RSA keys to be exactly 2048 bits, aligning them with the specification. - -Some code existed to allow tests to shorten this and it's -conceptually possible a modified client produced mutables -with different key-sizes. However, the spec says that they -must be 2048 bits. If you happen to have a capability with -a key-size different from 2048 you may use 1.17.1 or earlier -to read the content. diff --git a/newsfragments/3858.minor b/newsfragments/3858.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3859.minor b/newsfragments/3859.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3860.minor b/newsfragments/3860.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3865.incompat b/newsfragments/3865.incompat deleted file mode 100644 index 59381b269..000000000 --- a/newsfragments/3865.incompat +++ /dev/null @@ -1 +0,0 @@ -Python 3.6 is no longer supported, as it has reached end-of-life and is no longer receiving security updates. \ No newline at end of file diff --git a/newsfragments/3867.minor b/newsfragments/3867.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3868.minor b/newsfragments/3868.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3871.minor b/newsfragments/3871.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3873.incompat b/newsfragments/3873.incompat deleted file mode 100644 index da8a5fb0e..000000000 --- a/newsfragments/3873.incompat +++ /dev/null @@ -1 +0,0 @@ -Python 3.7 or later is now required; Python 2 is no longer supported. \ No newline at end of file diff --git a/newsfragments/3875.minor b/newsfragments/3875.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3876.minor b/newsfragments/3876.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3877.minor b/newsfragments/3877.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3879.incompat b/newsfragments/3879.incompat deleted file mode 100644 index ca3f24f94..000000000 --- a/newsfragments/3879.incompat +++ /dev/null @@ -1 +0,0 @@ -Share corruption reports stored on disk are now always encoded in UTF-8. \ No newline at end of file diff --git a/newsfragments/3881.minor b/newsfragments/3881.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3882.minor b/newsfragments/3882.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3883.minor b/newsfragments/3883.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3889.minor b/newsfragments/3889.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3890.minor b/newsfragments/3890.minor deleted file mode 100644 index e69de29bb..000000000 diff --git a/newsfragments/3902.feature b/newsfragments/3902.feature new file mode 100644 index 000000000..2477d0ae6 --- /dev/null +++ b/newsfragments/3902.feature @@ -0,0 +1 @@ +The new HTTPS-based storage server is now enabled transparently on the same port as the Foolscap server. This will not have any user-facing impact until the HTTPS storage protocol is supported in clients as well. \ No newline at end of file diff --git a/newsfragments/3788.minor b/newsfragments/3904.minor similarity index 100% rename from newsfragments/3788.minor rename to newsfragments/3904.minor diff --git a/newsfragments/3922.documentation b/newsfragments/3922.documentation new file mode 100644 index 000000000..d0232dd02 --- /dev/null +++ b/newsfragments/3922.documentation @@ -0,0 +1 @@ +Several minor errors in the Great Black Swamp proposed specification document have been fixed. \ No newline at end of file diff --git a/newsfragments/3802.minor b/newsfragments/3927.minor similarity index 100% rename from newsfragments/3802.minor rename to newsfragments/3927.minor diff --git a/newsfragments/3816.minor b/newsfragments/3928.minor similarity index 100% rename from newsfragments/3816.minor rename to newsfragments/3928.minor diff --git a/newsfragments/3938.bugfix b/newsfragments/3938.bugfix new file mode 100644 index 000000000..c2778cfdf --- /dev/null +++ b/newsfragments/3938.bugfix @@ -0,0 +1 @@ +Work with (and require) newer versions of pycddl. \ No newline at end of file diff --git a/newsfragments/3855.minor b/newsfragments/3940.minor similarity index 100% rename from newsfragments/3855.minor rename to newsfragments/3940.minor diff --git a/nix/sources.json b/nix/sources.json index 79eabe7a1..950151416 100644 --- a/nix/sources.json +++ b/nix/sources.json @@ -53,10 +53,10 @@ "homepage": "", "owner": "DavHau", "repo": "pypi-deps-db", - "rev": "76b8f1e44a8ec051b853494bcf3cc8453a294a6a", - "sha256": "18fgqyh4z578jjhk26n1xi2cw2l98vrqp962rgz9a6wa5yh1nm4x", + "rev": "5fe7d2d1c85cd86d64f4f079eef3f1ff5653bcd6", + "sha256": "0pc6mj7rzvmhh303rvj5wf4hrksm4h2rf4fsvqs0ljjdmgxrqm3f", "type": "tarball", - "url": "https://github.com/DavHau/pypi-deps-db/archive/76b8f1e44a8ec051b853494bcf3cc8453a294a6a.tar.gz", + "url": "https://github.com/DavHau/pypi-deps-db/archive/5fe7d2d1c85cd86d64f4f079eef3f1ff5653bcd6.tar.gz", "url_template": "https://github.com///archive/.tar.gz" } } diff --git a/relnotes.txt b/relnotes.txt index e9b298771..dd7cc9429 100644 --- a/relnotes.txt +++ b/relnotes.txt @@ -1,6 +1,6 @@ -ANNOUNCING Tahoe, the Least-Authority File Store, v1.17.1 +ANNOUNCING Tahoe, the Least-Authority File Store, v1.18.0 -The Tahoe-LAFS team is pleased to announce version 1.17.1 of +The Tahoe-LAFS team is pleased to announce version 1.18.0 of Tahoe-LAFS, an extremely reliable decentralized storage system. Get it with "pip install tahoe-lafs", or download a tarball here: @@ -15,10 +15,12 @@ unique security and fault-tolerance properties: https://tahoe-lafs.readthedocs.org/en/latest/about.html -The previous stable release of Tahoe-LAFS was v1.17.0, released on -December 6, 2021. +The previous stable release of Tahoe-LAFS was v1.17.1, released on +January 7, 2022. -This release fixes two Python3-releated regressions and 4 minor bugs. +This release drops support for Python 2 and for Python 3.6 and earlier. +twistd.pid is no longer used (in favour of one with pid + process creation time). +A collection of minor bugs and issues were also fixed. Please see ``NEWS.rst`` [1] for a complete list of changes. @@ -132,24 +134,23 @@ Of Fame" [13]. ACKNOWLEDGEMENTS -This is the nineteenth release of Tahoe-LAFS to be created -solely as a labor of love by volunteers. Thank you very much -to the team of "hackers in the public interest" who make -Tahoe-LAFS possible. +This is the twentieth release of Tahoe-LAFS to be created solely as a +labor of love by volunteers. Thank you very much to the team of +"hackers in the public interest" who make Tahoe-LAFS possible. meejah on behalf of the Tahoe-LAFS team -January 7, 2022 +October 1, 2022 Planet Earth -[1] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.17.1/NEWS.rst +[1] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.18.0/NEWS.rst [2] https://github.com/tahoe-lafs/tahoe-lafs/blob/master/docs/known_issues.rst [3] https://tahoe-lafs.org/trac/tahoe-lafs/wiki/RelatedProjects -[4] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.17.1/COPYING.GPL -[5] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.17.1/COPYING.TGPPL.rst -[6] https://tahoe-lafs.readthedocs.org/en/tahoe-lafs-1.17.1/INSTALL.html +[4] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.18.0/COPYING.GPL +[5] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.18.0/COPYING.TGPPL.rst +[6] https://tahoe-lafs.readthedocs.org/en/tahoe-lafs-1.18.0/INSTALL.html [7] https://lists.tahoe-lafs.org/mailman/listinfo/tahoe-dev [8] https://tahoe-lafs.org/trac/tahoe-lafs/roadmap [9] https://github.com/tahoe-lafs/tahoe-lafs/blob/master/CREDITS diff --git a/setup.py b/setup.py index b14893712..78f7042a9 100644 --- a/setup.py +++ b/setup.py @@ -114,9 +114,7 @@ install_requires = [ "attrs >= 18.2.0", # WebSocket library for twisted and asyncio - "autobahn >= 19.5.2, != 22.5.1, != 22.4.2, != 22.4.1" - # (the ignored versions above don't have autobahn.twisted.testing - # packaged properly) + "autobahn >= 22.4.3", # Support for Python 3 transition "future >= 0.18.2", @@ -135,10 +133,15 @@ install_requires = [ # HTTP server and client "klein", - "werkzeug", + # 2.2.0 has a bug: https://github.com/pallets/werkzeug/issues/2465 + "werkzeug != 2.2.0", "treq", "cbor2", - "pycddl", + "pycddl >= 0.2", + + # for pid-file support + "psutil", + "filelock", ] setup_requires = [ @@ -377,8 +380,15 @@ setup(name="tahoe-lafs", # also set in __init__.py # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2392 for some # discussion. ':sys_platform=="win32"': ["pywin32 != 226"], + "build": [ + "dulwich", + "gpg", + ], "test": [ "flake8", + # On Python 3.7, importlib_metadata v5 breaks flake8. + # https://github.com/python/importlib_metadata/issues/407 + "importlib_metadata<5; python_version < '3.8'", # Pin a specific pyflakes so we don't have different folks # disagreeing on what is or is not a lint issue. We can bump # this version from time to time, but we will do it diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 56ecdc6ed..1a158a1aa 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -1,17 +1,9 @@ """ Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, max, min # noqa: F401 - # Don't use future str to prevent leaking future's newbytes into foolscap, which they break. - from past.builtins import unicode as str +from __future__ import annotations +from typing import Optional import os, stat, time, weakref from base64 import urlsafe_b64encode from functools import partial @@ -112,6 +104,7 @@ _client_config = configutil.ValidConfiguration( "reserved_space", "storage_dir", "plugins", + "force_foolscap", ), "sftpd": ( "accounts.file", @@ -591,6 +584,10 @@ def anonymous_storage_enabled(config): @implementer(IStatsProducer) class _Client(node.Node, pollmixin.PollMixin): + """ + This class should be refactored; see + https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3931 + """ STOREDIR = 'storage' NODETYPE = "client" @@ -658,6 +655,14 @@ class _Client(node.Node, pollmixin.PollMixin): if webport: self.init_web(webport) # strports string + # TODO this may be the wrong location for now? but as temporary measure + # it allows us to get NURLs for testing in test_istorageserver.py. This + # will eventually get fixed one way or another in + # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3901. See also + # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3931 for the bigger + # picture issue. + self.storage_nurls : Optional[set] = None + def init_stats_provider(self): self.stats_provider = StatsProvider(self) self.stats_provider.setServiceParent(self) @@ -818,6 +823,11 @@ class _Client(node.Node, pollmixin.PollMixin): if anonymous_storage_enabled(self.config): furl_file = self.config.get_private_path("storage.furl").encode(get_filesystem_encoding()) furl = self.tub.registerReference(FoolscapStorageServer(ss), furlFile=furl_file) + (_, _, swissnum) = decode_furl(furl) + if hasattr(self.tub.negotiationClass, "add_storage_server"): + nurls = self.tub.negotiationClass.add_storage_server(ss, swissnum.encode("ascii")) + self.storage_nurls = nurls + announcement[storage_client.ANONYMOUS_STORAGE_NURLS] = [n.to_text() for n in nurls] announcement["anonymous-storage-FURL"] = furl enabled_storage_servers = self._enable_storage_servers( diff --git a/src/allmydata/history.py b/src/allmydata/history.py index b5cfb7318..06a22ab5d 100644 --- a/src/allmydata/history.py +++ b/src/allmydata/history.py @@ -20,7 +20,7 @@ class History(object): MAX_UPLOAD_STATUSES = 10 MAX_MAPUPDATE_STATUSES = 20 MAX_PUBLISH_STATUSES = 20 - MAX_RETRIEVE_STATUSES = 20 + MAX_RETRIEVE_STATUSES = 40 def __init__(self, stats_provider=None): self.stats_provider = stats_provider diff --git a/src/allmydata/immutable/encode.py b/src/allmydata/immutable/encode.py index 42fc18077..874492785 100644 --- a/src/allmydata/immutable/encode.py +++ b/src/allmydata/immutable/encode.py @@ -694,3 +694,24 @@ class Encoder(object): return self.uri_extension_data def get_uri_extension_hash(self): return self.uri_extension_hash + + def get_uri_extension_size(self): + """ + Calculate the size of the URI extension that gets written at the end of + immutables. + + This may be done earlier than actual encoding, so e.g. we might not + know the crypttext hashes, but that's fine for our purposes since we + only care about the length. + """ + params = self.uri_extension_data.copy() + params["crypttext_hash"] = b"\x00" * hashutil.CRYPTO_VAL_SIZE + params["crypttext_root_hash"] = b"\x00" * hashutil.CRYPTO_VAL_SIZE + params["share_root_hash"] = b"\x00" * hashutil.CRYPTO_VAL_SIZE + assert params.keys() == { + "codec_name", "codec_params", "size", "segment_size", "num_segments", + "needed_shares", "total_shares", "tail_codec_params", + "crypttext_hash", "crypttext_root_hash", "share_root_hash" + }, params.keys() + uri_extension = uri.pack_extension(params) + return len(uri_extension) diff --git a/src/allmydata/immutable/layout.py b/src/allmydata/immutable/layout.py index 79c886237..d552d43c4 100644 --- a/src/allmydata/immutable/layout.py +++ b/src/allmydata/immutable/layout.py @@ -19,6 +19,7 @@ from allmydata.util import mathutil, observer, pipeline, log from allmydata.util.assertutil import precondition from allmydata.storage.server import si_b2a + class LayoutInvalid(Exception): """ There is something wrong with these bytes so they can't be interpreted as the kind of immutable file that I know how to download.""" @@ -90,7 +91,7 @@ FORCE_V2 = False # set briefly by unit tests to make small-sized V2 shares def make_write_bucket_proxy(rref, server, data_size, block_size, num_segments, - num_share_hashes, uri_extension_size_max): + num_share_hashes, uri_extension_size): # Use layout v1 for small files, so they'll be readable by older versions # (= 2**32 or data_size >= 2**32: @@ -195,6 +196,14 @@ class WriteBucketProxy(object): return self._write(offset, data) def put_crypttext_hashes(self, hashes): + # plaintext_hash_tree precedes crypttext_hash_tree. It is not used, and + # so is not explicitly written, but we need to write everything, so + # fill it in with nulls. + d = self._write(self._offsets['plaintext_hash_tree'], b"\x00" * self._segment_hash_size) + d.addCallback(lambda _: self._really_put_crypttext_hashes(hashes)) + return d + + def _really_put_crypttext_hashes(self, hashes): offset = self._offsets['crypttext_hash_tree'] assert isinstance(hashes, list) data = b"".join(hashes) @@ -233,8 +242,7 @@ class WriteBucketProxy(object): def put_uri_extension(self, data): offset = self._offsets['uri_extension'] assert isinstance(data, bytes) - precondition(len(data) <= self._uri_extension_size_max, - len(data), self._uri_extension_size_max) + precondition(len(data) == self._uri_extension_size) length = struct.pack(self.fieldstruct, len(data)) return self._write(offset, length+data) @@ -244,11 +252,12 @@ class WriteBucketProxy(object): # would reduce the foolscap CPU overhead per share, but wouldn't # reduce the number of round trips, so it might not be worth the # effort. - + self._written_bytes += len(data) return self._pipeline.add(len(data), self._rref.callRemote, "write", offset, data) def close(self): + assert self._written_bytes == self.get_allocated_size(), f"{self._written_bytes} != {self.get_allocated_size()}" d = self._pipeline.add(0, self._rref.callRemote, "close") d.addCallback(lambda ign: self._pipeline.flush()) return d @@ -303,8 +312,6 @@ class WriteBucketProxy_v2(WriteBucketProxy): @implementer(IStorageBucketReader) class ReadBucketProxy(object): - MAX_UEB_SIZE = 2000 # actual size is closer to 419, but varies by a few bytes - def __init__(self, rref, server, storage_index): self._rref = rref self._server = server @@ -332,11 +339,6 @@ class ReadBucketProxy(object): # TODO: for small shares, read the whole bucket in _start() d = self._fetch_header() d.addCallback(self._parse_offsets) - # XXX The following two callbacks implement a slightly faster/nicer - # way to get the ueb and sharehashtree, but it requires that the - # storage server be >= v1.3.0. - # d.addCallback(self._fetch_sharehashtree_and_ueb) - # d.addCallback(self._parse_sharehashtree_and_ueb) def _fail_waiters(f): self._ready.fire(f) def _notify_waiters(result): @@ -381,29 +383,6 @@ class ReadBucketProxy(object): self._offsets[field] = offset return self._offsets - def _fetch_sharehashtree_and_ueb(self, offsets): - sharehashtree_size = offsets['uri_extension'] - offsets['share_hashes'] - return self._read(offsets['share_hashes'], - self.MAX_UEB_SIZE+sharehashtree_size) - - def _parse_sharehashtree_and_ueb(self, data): - sharehashtree_size = self._offsets['uri_extension'] - self._offsets['share_hashes'] - if len(data) < sharehashtree_size: - raise LayoutInvalid("share hash tree truncated -- should have at least %d bytes -- not %d" % (sharehashtree_size, len(data))) - if sharehashtree_size % (2+HASH_SIZE) != 0: - raise LayoutInvalid("share hash tree malformed -- should have an even multiple of %d bytes -- not %d" % (2+HASH_SIZE, sharehashtree_size)) - self._share_hashes = [] - for i in range(0, sharehashtree_size, 2+HASH_SIZE): - hashnum = struct.unpack(">H", data[i:i+2])[0] - hashvalue = data[i+2:i+2+HASH_SIZE] - self._share_hashes.append( (hashnum, hashvalue) ) - - i = self._offsets['uri_extension']-self._offsets['share_hashes'] - if len(data) < i+self._fieldsize: - raise LayoutInvalid("not enough bytes to encode URI length -- should be at least %d bytes long, not %d " % (i+self._fieldsize, len(data),)) - length = struct.unpack(self._fieldstruct, data[i:i+self._fieldsize])[0] - self._ueb_data = data[i+self._fieldsize:i+self._fieldsize+length] - def _get_block_data(self, unused, blocknum, blocksize, thisblocksize): offset = self._offsets['data'] + blocknum * blocksize return self._read(offset, thisblocksize) @@ -446,20 +425,18 @@ class ReadBucketProxy(object): else: return defer.succeed([]) - def _get_share_hashes(self, unused=None): - if hasattr(self, '_share_hashes'): - return self._share_hashes - return self._get_share_hashes_the_old_way() - def get_share_hashes(self): d = self._start_if_needed() d.addCallback(self._get_share_hashes) return d - def _get_share_hashes_the_old_way(self): + def _get_share_hashes(self, _ignore): """ Tahoe storage servers < v1.3.0 would return an error if you tried to read past the end of the share, so we need to use the offset and - read just that much.""" + read just that much. + + HTTP-based storage protocol also doesn't like reading past the end. + """ offset = self._offsets['share_hashes'] size = self._offsets['uri_extension'] - offset if size % (2+HASH_SIZE) != 0: @@ -477,32 +454,29 @@ class ReadBucketProxy(object): d.addCallback(_unpack_share_hashes) return d - def _get_uri_extension_the_old_way(self, unused=None): + def _get_uri_extension(self, unused=None): """ Tahoe storage servers < v1.3.0 would return an error if you tried to read past the end of the share, so we need to fetch the UEB size - and then read just that much.""" + and then read just that much. + + HTTP-based storage protocol also doesn't like reading past the end. + """ offset = self._offsets['uri_extension'] d = self._read(offset, self._fieldsize) def _got_length(data): if len(data) != self._fieldsize: raise LayoutInvalid("not enough bytes to encode URI length -- should be %d bytes long, not %d " % (self._fieldsize, len(data),)) length = struct.unpack(self._fieldstruct, data)[0] - if length >= 2**31: - # URI extension blocks are around 419 bytes long, so this - # must be corrupted. Anyway, the foolscap interface schema - # for "read" will not allow >= 2**31 bytes length. + if length >= 2000: + # URI extension blocks are around 419 bytes long; in previous + # versions of the code 1000 was used as a default catchall. So + # 2000 or more must be corrupted. raise RidiculouslyLargeURIExtensionBlock(length) return self._read(offset+self._fieldsize, length) d.addCallback(_got_length) return d - def _get_uri_extension(self, unused=None): - if hasattr(self, '_ueb_data'): - return self._ueb_data - else: - return self._get_uri_extension_the_old_way() - def get_uri_extension(self): d = self._start_if_needed() d.addCallback(self._get_uri_extension) diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index cb332dfdf..6b9b48f6a 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -242,31 +242,26 @@ class UploadResults(object): def get_verifycapstr(self): return self._verifycapstr -# our current uri_extension is 846 bytes for small files, a few bytes -# more for larger ones (since the filesize is encoded in decimal in a -# few places). Ask for a little bit more just in case we need it. If -# the extension changes size, we can change EXTENSION_SIZE to -# allocate a more accurate amount of space. -EXTENSION_SIZE = 1000 -# TODO: actual extensions are closer to 419 bytes, so we can probably lower -# this. def pretty_print_shnum_to_servers(s): return ', '.join([ "sh%s: %s" % (k, '+'.join([idlib.shortnodeid_b2a(x) for x in v])) for k, v in s.items() ]) + class ServerTracker(object): def __init__(self, server, sharesize, blocksize, num_segments, num_share_hashes, storage_index, - bucket_renewal_secret, bucket_cancel_secret): + bucket_renewal_secret, bucket_cancel_secret, + uri_extension_size): self._server = server self.buckets = {} # k: shareid, v: IRemoteBucketWriter self.sharesize = sharesize + self.uri_extension_size = uri_extension_size wbp = layout.make_write_bucket_proxy(None, None, sharesize, blocksize, num_segments, num_share_hashes, - EXTENSION_SIZE) + uri_extension_size) self.wbp_class = wbp.__class__ # to create more of them self.allocated_size = wbp.get_allocated_size() self.blocksize = blocksize @@ -314,7 +309,7 @@ class ServerTracker(object): self.blocksize, self.num_segments, self.num_share_hashes, - EXTENSION_SIZE) + self.uri_extension_size) b[sharenum] = bp self.buckets.update(b) return (alreadygot, set(b.keys())) @@ -487,7 +482,7 @@ class Tahoe2ServerSelector(log.PrefixingLogMixin): def get_shareholders(self, storage_broker, secret_holder, storage_index, share_size, block_size, num_segments, total_shares, needed_shares, - min_happiness): + min_happiness, uri_extension_size): """ @return: (upload_trackers, already_serverids), where upload_trackers is a set of ServerTracker instances that have agreed to hold @@ -529,7 +524,8 @@ class Tahoe2ServerSelector(log.PrefixingLogMixin): # figure out how much space to ask for wbp = layout.make_write_bucket_proxy(None, None, share_size, 0, num_segments, - num_share_hashes, EXTENSION_SIZE) + num_share_hashes, + uri_extension_size) allocated_size = wbp.get_allocated_size() # decide upon the renewal/cancel secrets, to include them in the @@ -554,7 +550,7 @@ class Tahoe2ServerSelector(log.PrefixingLogMixin): def _create_server_tracker(server, renew, cancel): return ServerTracker( server, share_size, block_size, num_segments, num_share_hashes, - storage_index, renew, cancel, + storage_index, renew, cancel, uri_extension_size ) readonly_trackers, write_trackers = self._create_trackers( @@ -1326,7 +1322,8 @@ class CHKUploader(object): d = server_selector.get_shareholders(storage_broker, secret_holder, storage_index, share_size, block_size, - num_segments, n, k, desired) + num_segments, n, k, desired, + encoder.get_uri_extension_size()) def _done(res): self._server_selection_elapsed = time.time() - server_selection_started return res diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 3ac4c507b..8266fe3fb 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -55,6 +55,8 @@ from allmydata.util.yamlutil import ( from . import ( __full_version__, ) +from .protocol_switch import create_tub_with_https_support + def _common_valid_config(): return configutil.ValidConfiguration({ @@ -695,7 +697,7 @@ def create_connection_handlers(config, i2p_provider, tor_provider): def create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers, - handler_overrides={}, **kwargs): + handler_overrides={}, force_foolscap=False, **kwargs): """ Create a Tub with the right options and handlers. It will be ephemeral unless the caller provides certFile= in kwargs @@ -705,8 +707,17 @@ def create_tub(tub_options, default_connection_handlers, foolscap_connection_han :param dict tub_options: every key-value pair in here will be set in the new Tub via `Tub.setOption` + + :param bool force_foolscap: If True, only allow Foolscap, not just HTTPS + storage protocol. """ - tub = Tub(**kwargs) + # We listen simultaneously for both Foolscap and HTTPS on the same port, + # so we have to create a special Foolscap Tub for that to work: + if force_foolscap: + tub = Tub(**kwargs) + else: + tub = create_tub_with_https_support(**kwargs) + for (name, value) in list(tub_options.items()): tub.setOption(name, value) handlers = default_connection_handlers.copy() @@ -896,14 +907,20 @@ def create_main_tub(config, tub_options, # FIXME? "node.pem" was the CERTFILE option/thing certfile = config.get_private_path("node.pem") - tub = create_tub( tub_options, default_connection_handlers, foolscap_connection_handlers, + # TODO eventually we will want the default to be False, but for now we + # don't want to enable HTTP by default. + # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3934 + force_foolscap=config.get_config( + "storage", "force_foolscap", default=True, boolean=True + ), handler_overrides=handler_overrides, certFile=certfile, ) + if portlocation is None: log.msg("Tub is not listening") else: diff --git a/src/allmydata/protocol_switch.py b/src/allmydata/protocol_switch.py new file mode 100644 index 000000000..b0af84c33 --- /dev/null +++ b/src/allmydata/protocol_switch.py @@ -0,0 +1,210 @@ +""" +Support for listening with both HTTPS and Foolscap on the same port. + +The goal is to make the transition from Foolscap to HTTPS-based protocols as +simple as possible, with no extra configuration needed. Listening on the same +port means a user upgrading Tahoe-LAFS will automatically get HTTPS working +with no additional changes. + +Use ``create_tub_with_https_support()`` creates a new ``Tub`` that has its +``negotiationClass`` modified to be a new subclass tied to that specific +``Tub`` instance. Calling ``tub.negotiationClass.add_storage_server(...)`` +then adds relevant information for a storage server once it becomes available +later in the configuration process. +""" + +from __future__ import annotations + +from itertools import chain + +from twisted.internet.protocol import Protocol +from twisted.internet.interfaces import IDelayedCall +from twisted.internet.ssl import CertificateOptions +from twisted.web.server import Site +from twisted.protocols.tls import TLSMemoryBIOFactory +from twisted.internet import reactor + +from hyperlink import DecodedURL +from foolscap.negotiate import Negotiation +from foolscap.api import Tub + +from .storage.http_server import HTTPServer, build_nurl +from .storage.server import StorageServer + + +class _PretendToBeNegotiation(type): + """ + Metaclass that allows ``_FoolscapOrHttps`` to pretend to be a + ``Negotiation`` instance, since Foolscap does some checks like + ``assert isinstance(protocol, tub.negotiationClass)`` in its internals, + and sometimes that ``protocol`` is a ``_FoolscapOrHttps`` instance, but + sometimes it's a ``Negotiation`` instance. + """ + + def __instancecheck__(self, instance): + return issubclass(instance.__class__, self) or isinstance(instance, Negotiation) + + +class _FoolscapOrHttps(Protocol, metaclass=_PretendToBeNegotiation): + """ + Based on initial query, decide whether we're talking Foolscap or HTTP. + + Additionally, pretends to be a ``foolscap.negotiate.Negotiation`` instance, + since these are created by Foolscap's ``Tub``, by setting this to be the + tub's ``negotiationClass``. + + Do not instantiate directly, use ``create_tub_with_https_support(...)`` + instead. The way this class works is that a new subclass is created for a + specific ``Tub`` instance. + """ + + # These are class attributes; they will be set by + # create_tub_with_https_support() and add_storage_server(). + + # The Twisted HTTPS protocol factory wrapping the storage server HTTP API: + https_factory: TLSMemoryBIOFactory + # The tub that created us: + tub: Tub + + @classmethod + def add_storage_server( + cls, storage_server: StorageServer, swissnum: bytes + ) -> set[DecodedURL]: + """ + Update a ``_FoolscapOrHttps`` subclass for a specific ``Tub`` instance + with the class attributes it requires for a specific storage server. + + Returns the resulting NURLs. + """ + # We need to be a subclass: + assert cls != _FoolscapOrHttps + # The tub instance must already be set: + assert hasattr(cls, "tub") + assert isinstance(cls.tub, Tub) + + # Tub.myCertificate is a twisted.internet.ssl.PrivateCertificate + # instance. + certificate_options = CertificateOptions( + privateKey=cls.tub.myCertificate.privateKey.original, + certificate=cls.tub.myCertificate.original, + ) + + http_storage_server = HTTPServer(storage_server, swissnum) + cls.https_factory = TLSMemoryBIOFactory( + certificate_options, + False, + Site(http_storage_server.get_resource()), + ) + + storage_nurls = set() + # Individual hints can be in the form + # "tcp:host:port,tcp:host:port,tcp:host:port". + for location_hint in chain.from_iterable( + hints.split(",") for hints in cls.tub.locationHints + ): + if location_hint.startswith("tcp:"): + _, hostname, port = location_hint.split(":") + port = int(port) + storage_nurls.add( + build_nurl( + hostname, + port, + str(swissnum, "ascii"), + cls.tub.myCertificate.original.to_cryptography(), + ) + ) + # TODO this is probably where we'll have to support Tor and I2P? + # See https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3888#comment:9 + # for discussion (there will be separate tickets added for those at + # some point.) + return storage_nurls + + def __init__(self, *args, **kwargs): + self._foolscap: Negotiation = Negotiation(*args, **kwargs) + + def __setattr__(self, name, value): + if name in {"_foolscap", "_buffer", "transport", "__class__", "_timeout"}: + object.__setattr__(self, name, value) + else: + setattr(self._foolscap, name, value) + + def __getattr__(self, name): + return getattr(self._foolscap, name) + + def _convert_to_negotiation(self): + """ + Convert self to a ``Negotiation`` instance. + """ + self.__class__ = Negotiation # type: ignore + self.__dict__ = self._foolscap.__dict__ + + def initClient(self, *args, **kwargs): + # After creation, a Negotiation instance either has initClient() or + # initServer() called. Since this is a client, we're never going to do + # HTTP, so we can immediately become a Negotiation instance. + assert not hasattr(self, "_buffer") + self._convert_to_negotiation() + return self.initClient(*args, **kwargs) + + def connectionMade(self): + self._buffer: bytes = b"" + self._timeout: IDelayedCall = reactor.callLater( + 30, self.transport.abortConnection + ) + + def connectionLost(self, reason): + if self._timeout.active(): + self._timeout.cancel() + + def dataReceived(self, data: bytes) -> None: + """Handle incoming data. + + Once we've decided which protocol we are, update self.__class__, at + which point all methods will be called on the new class. + """ + self._buffer += data + if len(self._buffer) < 8: + return + + # Check if it looks like a Foolscap request. If so, it can handle this + # and later data, otherwise assume HTTPS. + self._timeout.cancel() + if self._buffer.startswith(b"GET /id/"): + # We're a Foolscap Negotiation server protocol instance: + transport = self.transport + buf = self._buffer + self._convert_to_negotiation() + self.makeConnection(transport) + self.dataReceived(buf) + return + else: + # We're a HTTPS protocol instance, serving the storage protocol: + assert self.transport is not None + protocol = self.https_factory.buildProtocol(self.transport.getPeer()) + protocol.makeConnection(self.transport) + protocol.dataReceived(self._buffer) + + # Update the factory so it knows we're transforming to a new + # protocol object (we'll do that next) + value = self.https_factory.protocols.pop(protocol) + self.https_factory.protocols[self] = value + + # Transform self into the TLS protocol 🪄 + self.__class__ = protocol.__class__ + self.__dict__ = protocol.__dict__ + + +def create_tub_with_https_support(**kwargs) -> Tub: + """ + Create a new Tub that also supports HTTPS. + + This involves creating a new protocol switch class for the specific ``Tub`` + instance. + """ + the_tub = Tub(**kwargs) + + class FoolscapOrHttpForTub(_FoolscapOrHttps): + tub = the_tub + + the_tub.negotiationClass = FoolscapOrHttpForTub # type: ignore + return the_tub diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py index a0d8a752b..756c26f2c 100644 --- a/src/allmydata/scripts/runner.py +++ b/src/allmydata/scripts/runner.py @@ -47,11 +47,6 @@ if _default_nodedir: NODEDIR_HELP += " [default for most commands: " + quote_local_unicode_path(_default_nodedir) + "]" -# XXX all this 'dispatch' stuff needs to be unified + fixed up -_control_node_dispatch = { - "run": tahoe_run.run, -} - process_control_commands = [ ("run", None, tahoe_run.RunOptions, "run a node without daemonizing"), ] # type: SubCommands @@ -195,6 +190,7 @@ def parse_or_exit(config, argv, stdout, stderr): return config def dispatch(config, + reactor, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr): command = config.subCommand so = config.subOptions @@ -206,8 +202,8 @@ def dispatch(config, if command in create_dispatch: f = create_dispatch[command] - elif command in _control_node_dispatch: - f = _control_node_dispatch[command] + elif command == "run": + f = lambda config: tahoe_run.run(reactor, config) elif command in debug.dispatch: f = debug.dispatch[command] elif command in admin.dispatch: @@ -361,7 +357,7 @@ def _run_with_reactor(reactor, config, argv, stdout, stderr): stderr, ) d.addCallback(_maybe_enable_eliot_logging, reactor) - d.addCallback(dispatch, stdout=stdout, stderr=stderr) + d.addCallback(dispatch, reactor, stdout=stdout, stderr=stderr) def _show_exception(f): # when task.react() notices a non-SystemExit exception, it does # log.err() with the failure and then exits with rc=1. We want this diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 51be32ee3..65d520f57 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -19,6 +19,7 @@ import os, sys from allmydata.scripts.common import BasedirOptions from twisted.scripts import twistd from twisted.python import usage +from twisted.python.filepath import FilePath from twisted.python.reflect import namedAny from twisted.internet.defer import maybeDeferred from twisted.application.service import Service @@ -27,6 +28,13 @@ from allmydata.scripts.default_nodedir import _default_nodedir from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_path from allmydata.util.configutil import UnknownConfigError from allmydata.util.deferredutil import HookMixin +from allmydata.util.pid import ( + parse_pidfile, + check_pid_process, + cleanup_pidfile, + ProcessInTheWay, + InvalidPidFile, +) from allmydata.storage.crawler import ( MigratePickleFileError, ) @@ -35,35 +43,34 @@ from allmydata.node import ( PrivacyError, ) + def get_pidfile(basedir): """ Returns the path to the PID file. :param basedir: the node's base directory :returns: the path to the PID file """ - return os.path.join(basedir, u"twistd.pid") + return os.path.join(basedir, u"running.process") + def get_pid_from_pidfile(pidfile): """ Tries to read and return the PID stored in the node's PID file - (twistd.pid). + :param pidfile: try to read this PID file :returns: A numeric PID on success, ``None`` if PID file absent or inaccessible, ``-1`` if PID file invalid. """ try: - with open(pidfile, "r") as f: - pid = f.read() + pid, _ = parse_pidfile(pidfile) except EnvironmentError: return None - - try: - pid = int(pid) - except ValueError: + except InvalidPidFile: return -1 return pid + def identify_node_type(basedir): """ :return unicode: None or one of: 'client' or 'introducer'. @@ -179,7 +186,7 @@ class DaemonizeTheRealService(Service, HookMixin): ) ) else: - self.stderr.write("\nUnknown error\n") + self.stderr.write("\nUnknown error, here's the traceback:\n") reason.printTraceback(self.stderr) reactor.stop() @@ -206,7 +213,7 @@ class DaemonizeTahoeNodePlugin(object): return DaemonizeTheRealService(self.nodetype, self.basedir, so) -def run(config, runApp=twistd.runApp): +def run(reactor, config, runApp=twistd.runApp): """ Runs a Tahoe-LAFS node in the foreground. @@ -227,10 +234,15 @@ def run(config, runApp=twistd.runApp): print("%s is not a recognizable node directory" % quoted_basedir, file=err) return 1 - twistd_args = ["--nodaemon", "--rundir", basedir] + twistd_args = [ + # ensure twistd machinery does not daemonize. + "--nodaemon", + "--rundir", basedir, + ] if sys.platform != "win32": - pidfile = get_pidfile(basedir) - twistd_args.extend(["--pidfile", pidfile]) + # turn off Twisted's pid-file to use our own -- but not on + # windows, because twistd doesn't know about pidfiles there + twistd_args.extend(["--pidfile", None]) twistd_args.extend(config.twistd_args) twistd_args.append("DaemonizeTahoeNode") # point at our DaemonizeTahoeNodePlugin @@ -246,10 +258,18 @@ def run(config, runApp=twistd.runApp): return 1 twistd_config.loadedPlugins = {"DaemonizeTahoeNode": DaemonizeTahoeNodePlugin(nodetype, basedir)} - # handle invalid PID file (twistd might not start otherwise) - if sys.platform != "win32" and get_pid_from_pidfile(pidfile) == -1: - print("found invalid PID file in %s - deleting it" % basedir, file=err) - os.remove(pidfile) + # our own pid-style file contains PID and process creation time + pidfile = FilePath(get_pidfile(config['basedir'])) + try: + check_pid_process(pidfile) + except (ProcessInTheWay, InvalidPidFile) as e: + print("ERROR: {}".format(e), file=err) + return 1 + else: + reactor.addSystemEventTrigger( + "after", "shutdown", + lambda: cleanup_pidfile(pidfile) + ) # We always pass --nodaemon so twistd.runApp does not daemonize. print("running node in %s" % (quoted_basedir,), file=out) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index da350e0c6..79bf061c9 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -4,10 +4,12 @@ HTTP client that talks to the HTTP storage server. from __future__ import annotations -from typing import Union, Optional, Sequence, Mapping +from typing import Union, Optional, Sequence, Mapping, BinaryIO from base64 import b64encode +from io import BytesIO +from os import SEEK_END -from attrs import define, asdict, frozen +from attrs import define, asdict, frozen, field # TODO Make sure to import Python version? from cbor2 import loads, dumps @@ -17,8 +19,12 @@ from werkzeug.datastructures import Range, ContentRange from twisted.web.http_headers import Headers from twisted.web import http from twisted.web.iweb import IPolicyForHTTPS -from twisted.internet.defer import inlineCallbacks, returnValue, fail, Deferred -from twisted.internet.interfaces import IOpenSSLClientConnectionCreator +from twisted.internet.defer import inlineCallbacks, returnValue, fail, Deferred, succeed +from twisted.internet.interfaces import ( + IOpenSSLClientConnectionCreator, + IReactorTime, + IDelayedCall, +) from twisted.internet.ssl import CertificateOptions from twisted.web.client import Agent, HTTPConnectionPool from zope.interface import implementer @@ -28,6 +34,7 @@ from treq.client import HTTPClient from treq.testing import StubTreq from OpenSSL import SSL from cryptography.hazmat.bindings.openssl.binding import Binding +from werkzeug.http import parse_content_range_header from .http_common import ( swissnum_auth_header, @@ -80,54 +87,94 @@ _SCHEMAS = { "allocate_buckets": Schema( """ response = { - already-have: #6.258([* uint]) - allocated: #6.258([* uint]) + already-have: #6.258([0*256 uint]) + allocated: #6.258([0*256 uint]) } """ ), "immutable_write_share_chunk": Schema( """ response = { - required: [* {begin: uint, end: uint}] + required: [0* {begin: uint, end: uint}] } """ ), "list_shares": Schema( """ - response = #6.258([* uint]) + response = #6.258([0*256 uint]) """ ), "mutable_read_test_write": Schema( """ response = { "success": bool, - "data": {* share_number: [* bstr]} + "data": {0*256 share_number: [0* bstr]} } share_number = uint """ ), + "mutable_list_shares": Schema( + """ + response = #6.258([0*256 uint]) + """ + ), } -def _decode_cbor(response, schema: Schema): - """Given HTTP response, return decoded CBOR body.""" +@define +class _LengthLimitedCollector: + """ + Collect data using ``treq.collect()``, with limited length. + """ - def got_content(data): - schema.validate_cbor(data) - return loads(data) + remaining_length: int + timeout_on_silence: IDelayedCall + f: BytesIO = field(factory=BytesIO) - if response.code > 199 and response.code < 300: - content_type = get_content_type(response.headers) - if content_type == CBOR_MIME_TYPE: - # TODO limit memory usage - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3872 - return treq.content(response).addCallback(got_content) - else: - raise ClientException(-1, "Server didn't send CBOR") - else: - return treq.content(response).addCallback( - lambda data: fail(ClientException(response.code, response.phrase, data)) - ) + def __call__(self, data: bytes): + self.timeout_on_silence.reset(60) + self.remaining_length -= len(data) + if self.remaining_length < 0: + raise ValueError("Response length was too long") + self.f.write(data) + + +def limited_content( + response, + clock: IReactorTime, + max_length: int = 30 * 1024 * 1024, +) -> Deferred[BinaryIO]: + """ + Like ``treq.content()``, but limit data read from the response to a set + length. If the response is longer than the max allowed length, the result + fails with a ``ValueError``. + + A potentially useful future improvement would be using a temporary file to + store the content; since filesystem buffering means that would use memory + for small responses and disk for large responses. + + This will time out if no data is received for 60 seconds; so long as a + trickle of data continues to arrive, it will continue to run. + """ + d = succeed(None) + timeout = clock.callLater(60, d.cancel) + collector = _LengthLimitedCollector(max_length, timeout) + + # Make really sure everything gets called in Deferred context, treq might + # call collector directly... + d.addCallback(lambda _: treq.collect(response, collector)) + + def done(_): + timeout.cancel() + collector.f.seek(0) + return collector.f + + def failed(f): + if timeout.active(): + timeout.cancel() + return f + + return d.addCallbacks(done, failed) @define @@ -229,42 +276,67 @@ class _StorageClientHTTPSPolicy: ) -@define +@define(hash=True) class StorageClient(object): """ Low-level HTTP client that talks to the HTTP storage server. """ + # If set, we're doing unit testing and we should call this with + # HTTPConnectionPool we create. + TEST_MODE_REGISTER_HTTP_POOL = None + + @classmethod + def start_test_mode(cls, callback): + """Switch to testing mode. + + In testing mode we register the pool with test system using the given + callback so it can Do Things, most notably killing off idle HTTP + connections at test shutdown and, in some tests, in the midddle of the + test. + """ + cls.TEST_MODE_REGISTER_HTTP_POOL = callback + + @classmethod + def stop_test_mode(cls): + """Stop testing mode.""" + cls.TEST_MODE_REGISTER_HTTP_POOL = None + # The URL is a HTTPS URL ("https://..."). To construct from a NURL, use # ``StorageClient.from_nurl()``. _base_url: DecodedURL _swissnum: bytes _treq: Union[treq, StubTreq, HTTPClient] + _clock: IReactorTime @classmethod def from_nurl( - cls, nurl: DecodedURL, reactor, persistent: bool = True + cls, + nurl: DecodedURL, + reactor, ) -> StorageClient: """ Create a ``StorageClient`` for the given NURL. - - ``persistent`` indicates whether to use persistent HTTP connections. """ assert nurl.fragment == "v=1" assert nurl.scheme == "pb" swissnum = nurl.path[0].encode("ascii") certificate_hash = nurl.user.encode("ascii") + pool = HTTPConnectionPool(reactor) + + if cls.TEST_MODE_REGISTER_HTTP_POOL is not None: + cls.TEST_MODE_REGISTER_HTTP_POOL(pool) treq_client = HTTPClient( Agent( reactor, _StorageClientHTTPSPolicy(expected_spki_hash=certificate_hash), - pool=HTTPConnectionPool(reactor, persistent=persistent), + pool=pool, ) ) https_url = DecodedURL().replace(scheme="https", host=nurl.host, port=nurl.port) - return cls(https_url, swissnum, treq_client) + return cls(https_url, swissnum, treq_client, reactor) def relative_url(self, path): """Get a URL relative to the base URL.""" @@ -290,7 +362,8 @@ class StorageClient(object): write_enabler_secret=None, headers=None, message_to_serialize=None, - **kwargs + timeout: float = 60, + **kwargs, ): """ Like ``treq.request()``, but with optional secrets that get translated @@ -298,6 +371,8 @@ class StorageClient(object): If ``message_to_serialize`` is set, it will be serialized (by default with CBOR) and set as the request body. + + Default timeout is 60 seconds. """ headers = self._get_headers(headers) @@ -329,27 +404,75 @@ class StorageClient(object): kwargs["data"] = dumps(message_to_serialize) headers.addRawHeader("Content-Type", CBOR_MIME_TYPE) - return self._treq.request(method, url, headers=headers, **kwargs) + return self._treq.request( + method, url, headers=headers, timeout=timeout, **kwargs + ) + + def decode_cbor(self, response, schema: Schema): + """Given HTTP response, return decoded CBOR body.""" + + def got_content(f: BinaryIO): + data = f.read() + schema.validate_cbor(data) + return loads(data) + + if response.code > 199 and response.code < 300: + content_type = get_content_type(response.headers) + if content_type == CBOR_MIME_TYPE: + return limited_content(response, self._clock).addCallback(got_content) + else: + raise ClientException(-1, "Server didn't send CBOR") + else: + return treq.content(response).addCallback( + lambda data: fail(ClientException(response.code, response.phrase, data)) + ) +@define(hash=True) class StorageClientGeneral(object): """ High-level HTTP APIs that aren't immutable- or mutable-specific. """ - def __init__(self, client): # type: (StorageClient) -> None - self._client = client + _client: StorageClient @inlineCallbacks def get_version(self): """ Return the version metadata for the server. """ - url = self._client.relative_url("/v1/version") + url = self._client.relative_url("/storage/v1/version") response = yield self._client.request("GET", url) - decoded_response = yield _decode_cbor(response, _SCHEMAS["get_version"]) + decoded_response = yield self._client.decode_cbor( + response, _SCHEMAS["get_version"] + ) returnValue(decoded_response) + @inlineCallbacks + def add_or_renew_lease( + self, storage_index: bytes, renew_secret: bytes, cancel_secret: bytes + ) -> Deferred[None]: + """ + Add or renew a lease. + + If the renewal secret matches an existing lease, it is renewed. + Otherwise a new lease is added. + """ + url = self._client.relative_url( + "/storage/v1/lease/{}".format(_encode_si(storage_index)) + ) + response = yield self._client.request( + "PUT", + url, + lease_renew_secret=renew_secret, + lease_cancel_secret=cancel_secret, + ) + + if response.code == http.NO_CONTENT: + return + else: + raise ClientException(response.code) + @define class UploadProgress(object): @@ -375,35 +498,98 @@ def read_share_chunk( """ Download a chunk of data from a share. - TODO https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3857 Failed - downloads should be transparently retried and redownloaded by the - implementation a few times so that if a failure percolates up, the - caller can assume the failure isn't a short-term blip. + TODO https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3857 Failed downloads + should be transparently retried and redownloaded by the implementation a + few times so that if a failure percolates up, the caller can assume the + failure isn't a short-term blip. - NOTE: the underlying HTTP protocol is much more flexible than this API, - so a future refactor may expand this in order to simplify the calling - code and perhaps download data more efficiently. But then again maybe - the HTTP protocol will be simplified, see - https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3777 + NOTE: the underlying HTTP protocol is somewhat more flexible than this API, + insofar as it doesn't always require a range. In practice a range is + always provided by the current callers. """ url = client.relative_url( - "/v1/{}/{}/{}".format(share_type, _encode_si(storage_index), share_number) + "/storage/v1/{}/{}/{}".format( + share_type, _encode_si(storage_index), share_number + ) ) + # The default 60 second timeout is for getting the response, so it doesn't + # include the time it takes to download the body... so we will will deal + # with that later, via limited_content(). response = yield client.request( "GET", url, headers=Headers( + # Ranges in HTTP are _inclusive_, Python's convention is exclusive, + # but Range constructor does that the conversion for us. {"range": [Range("bytes", [(offset, offset + length)]).to_header()]} ), + unbuffered=True, # Don't buffer the response in memory. ) + + if response.code == http.NO_CONTENT: + return b"" + if response.code == http.PARTIAL_CONTENT: - body = yield response.content() - returnValue(body) + content_range = parse_content_range_header( + response.headers.getRawHeaders("content-range")[0] or "" + ) + if ( + content_range is None + or content_range.stop is None + or content_range.start is None + ): + raise ValueError( + "Content-Range was missing, invalid, or in format we don't support" + ) + supposed_length = content_range.stop - content_range.start + if supposed_length > length: + raise ValueError("Server sent more than we asked for?!") + # It might also send less than we asked for. That's (probably) OK, e.g. + # if we went past the end of the file. + body = yield limited_content(response, client._clock, supposed_length) + body.seek(0, SEEK_END) + actual_length = body.tell() + if actual_length != supposed_length: + # Most likely a mutable that got changed out from under us, but + # conceivably could be a bug... + raise ValueError( + f"Length of response sent from server ({actual_length}) " + + f"didn't match Content-Range header ({supposed_length})" + ) + body.seek(0) + return body.read() else: + # Technically HTTP allows sending an OK with full body under these + # circumstances, but the server is not designed to do that so we ignore + # that possibility for now... raise ClientException(response.code) -@define +@async_to_deferred +async def advise_corrupt_share( + client: StorageClient, + share_type: str, + storage_index: bytes, + share_number: int, + reason: str, +): + assert isinstance(reason, str) + url = client.relative_url( + "/storage/v1/{}/{}/{}/corrupt".format( + share_type, _encode_si(storage_index), share_number + ) + ) + message = {"reason": reason} + response = await client.request("POST", url, message_to_serialize=message) + if response.code == http.OK: + return + else: + raise ClientException( + response.code, + ) + + +@define(hash=True) class StorageClientImmutables(object): """ APIs for interacting with immutables. @@ -434,7 +620,9 @@ class StorageClientImmutables(object): Result fires when creating the storage index succeeded, if creating the storage index failed the result will fire with an exception. """ - url = self._client.relative_url("/v1/immutable/" + _encode_si(storage_index)) + url = self._client.relative_url( + "/storage/v1/immutable/" + _encode_si(storage_index) + ) message = {"share-numbers": share_numbers, "allocated-size": allocated_size} response = yield self._client.request( @@ -445,7 +633,9 @@ class StorageClientImmutables(object): upload_secret=upload_secret, message_to_serialize=message, ) - decoded_response = yield _decode_cbor(response, _SCHEMAS["allocate_buckets"]) + decoded_response = yield self._client.decode_cbor( + response, _SCHEMAS["allocate_buckets"] + ) returnValue( ImmutableCreateResult( already_have=decoded_response["already-have"], @@ -459,7 +649,9 @@ class StorageClientImmutables(object): ) -> Deferred[None]: """Abort the upload.""" url = self._client.relative_url( - "/v1/immutable/{}/{}/abort".format(_encode_si(storage_index), share_number) + "/storage/v1/immutable/{}/{}/abort".format( + _encode_si(storage_index), share_number + ) ) response = yield self._client.request( "PUT", @@ -491,7 +683,9 @@ class StorageClientImmutables(object): been uploaded. """ url = self._client.relative_url( - "/v1/immutable/{}/{}".format(_encode_si(storage_index), share_number) + "/storage/v1/immutable/{}/{}".format( + _encode_si(storage_index), share_number + ) ) response = yield self._client.request( "PATCH", @@ -517,7 +711,9 @@ class StorageClientImmutables(object): raise ClientException( response.code, ) - body = yield _decode_cbor(response, _SCHEMAS["immutable_write_share_chunk"]) + body = yield self._client.decode_cbor( + response, _SCHEMAS["immutable_write_share_chunk"] + ) remaining = RangeMap() for chunk in body["required"]: remaining.set(True, chunk["begin"], chunk["end"]) @@ -534,49 +730,23 @@ class StorageClientImmutables(object): ) @inlineCallbacks - def list_shares(self, storage_index): # type: (bytes,) -> Deferred[set[int]] + def list_shares(self, storage_index: bytes) -> Deferred[set[int]]: """ Return the set of shares for a given storage index. """ url = self._client.relative_url( - "/v1/immutable/{}/shares".format(_encode_si(storage_index)) + "/storage/v1/immutable/{}/shares".format(_encode_si(storage_index)) ) response = yield self._client.request( "GET", url, ) if response.code == http.OK: - body = yield _decode_cbor(response, _SCHEMAS["list_shares"]) + body = yield self._client.decode_cbor(response, _SCHEMAS["list_shares"]) returnValue(set(body)) else: raise ClientException(response.code) - @inlineCallbacks - def add_or_renew_lease( - self, storage_index: bytes, renew_secret: bytes, cancel_secret: bytes - ): - """ - Add or renew a lease. - - If the renewal secret matches an existing lease, it is renewed. - Otherwise a new lease is added. - """ - url = self._client.relative_url( - "/v1/lease/{}".format(_encode_si(storage_index)) - ) - response = yield self._client.request( - "PUT", - url, - lease_renew_secret=renew_secret, - lease_cancel_secret=cancel_secret, - ) - - if response.code == http.NO_CONTENT: - return - else: - raise ClientException(response.code) - - @inlineCallbacks def advise_corrupt_share( self, storage_index: bytes, @@ -584,20 +754,9 @@ class StorageClientImmutables(object): reason: str, ): """Indicate a share has been corrupted, with a human-readable message.""" - assert isinstance(reason, str) - url = self._client.relative_url( - "/v1/immutable/{}/{}/corrupt".format( - _encode_si(storage_index), share_number - ) + return advise_corrupt_share( + self._client, "immutable", storage_index, share_number, reason ) - message = {"reason": reason} - response = yield self._client.request("POST", url, message_to_serialize=message) - if response.code == http.OK: - return - else: - raise ClientException( - response.code, - ) @frozen @@ -631,8 +790,8 @@ class ReadVector: class TestWriteVectors: """Test and write vectors for a specific share.""" - test_vectors: Sequence[TestVector] - write_vectors: Sequence[WriteVector] + test_vectors: Sequence[TestVector] = field(factory=list) + write_vectors: Sequence[WriteVector] = field(factory=list) new_length: Optional[int] = None def asdict(self) -> dict: @@ -681,9 +840,8 @@ class StorageClientMutables: Given a mapping between share numbers and test/write vectors, the tests are done and if they are valid the writes are done. """ - # TODO unit test all the things url = self._client.relative_url( - "/v1/mutable/{}/read-test-write".format(_encode_si(storage_index)) + "/storage/v1/mutable/{}/read-test-write".format(_encode_si(storage_index)) ) message = { "test-write-vectors": { @@ -701,7 +859,9 @@ class StorageClientMutables: message_to_serialize=message, ) if response.code == http.OK: - result = await _decode_cbor(response, _SCHEMAS["mutable_read_test_write"]) + result = await self._client.decode_cbor( + response, _SCHEMAS["mutable_read_test_write"] + ) return ReadTestWriteResult(success=result["success"], reads=result["data"]) else: raise ClientException(response.code, (await response.content())) @@ -712,11 +872,37 @@ class StorageClientMutables: share_number: int, offset: int, length: int, - ) -> bytes: + ) -> Deferred[bytes]: """ Download a chunk of data from a share. """ - # TODO unit test all the things return read_share_chunk( self._client, "mutable", storage_index, share_number, offset, length ) + + @async_to_deferred + async def list_shares(self, storage_index: bytes) -> set[int]: + """ + List the share numbers for a given storage index. + """ + url = self._client.relative_url( + "/storage/v1/mutable/{}/shares".format(_encode_si(storage_index)) + ) + response = await self._client.request("GET", url) + if response.code == http.OK: + return await self._client.decode_cbor( + response, _SCHEMAS["mutable_list_shares"] + ) + else: + raise ClientException(response.code) + + def advise_corrupt_share( + self, + storage_index: bytes, + share_number: int, + reason: str, + ): + """Indicate a share has been corrupted, with a human-readable message.""" + return advise_corrupt_share( + self._client, "mutable", storage_index, share_number, reason + ) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 0169d1463..3902976ba 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -2,23 +2,31 @@ HTTP server for storage. """ -from typing import Dict, List, Set, Tuple, Any +from __future__ import annotations +from typing import Dict, List, Set, Tuple, Any, Callable, Union, cast from functools import wraps from base64 import b64decode import binascii +from tempfile import TemporaryFile +from cryptography.x509 import Certificate as CryptoCertificate from zope.interface import implementer from klein import Klein from twisted.web import http -from twisted.internet.interfaces import IListeningPort, IStreamServerEndpoint +from twisted.internet.interfaces import ( + IListeningPort, + IStreamServerEndpoint, + IPullProducer, +) +from twisted.internet.address import IPv4Address, IPv6Address from twisted.internet.defer import Deferred from twisted.internet.ssl import CertificateOptions, Certificate, PrivateCertificate -from twisted.web.server import Site +from twisted.web.server import Site, Request from twisted.protocols.tls import TLSMemoryBIOFactory from twisted.python.filepath import FilePath -import attr +from attrs import define, field, Factory from werkzeug.http import ( parse_range_header, parse_content_range_header, @@ -31,7 +39,7 @@ from cryptography.x509 import load_pem_x509_certificate # TODO Make sure to use pure Python versions? -from cbor2 import dumps, loads +from cbor2 import dump, loads from pycddl import Schema, ValidationError as CDDLValidationError from .server import StorageServer from .http_common import ( @@ -46,6 +54,7 @@ from .common import si_a2b from .immutable import BucketWriter, ConflictingWriteError from ..util.hashutil import timing_safe_compare from ..util.base32 import rfc3548_alphabet +from allmydata.interfaces import BadWriteEnablerError class ClientSecretsException(Exception): @@ -135,31 +144,31 @@ def _authorized_route(app, required_secrets, *route_args, **route_kwargs): return decorator -@attr.s +@define class StorageIndexUploads(object): """ In-progress upload to storage index. """ # Map share number to BucketWriter - shares = attr.ib(factory=dict) # type: Dict[int,BucketWriter] + shares: dict[int, BucketWriter] = Factory(dict) # Map share number to the upload secret (different shares might have # different upload secrets). - upload_secrets = attr.ib(factory=dict) # type: Dict[int,bytes] + upload_secrets: dict[int, bytes] = Factory(dict) -@attr.s +@define class UploadsInProgress(object): """ Keep track of uploads for storage indexes. """ # Map storage index to corresponding uploads-in-progress - _uploads = attr.ib(type=Dict[bytes, StorageIndexUploads], factory=dict) + _uploads: dict[bytes, StorageIndexUploads] = Factory(dict) # Map BucketWriter to (storage index, share number) - _bucketwriters = attr.ib(type=Dict[BucketWriter, Tuple[bytes, int]], factory=dict) + _bucketwriters: dict[BucketWriter, Tuple[bytes, int]] = Factory(dict) def add_write_bucket( self, @@ -186,7 +195,12 @@ class UploadsInProgress(object): def remove_write_bucket(self, bucket: BucketWriter): """Stop tracking the given ``BucketWriter``.""" - storage_index, share_number = self._bucketwriters.pop(bucket) + try: + storage_index, share_number = self._bucketwriters.pop(bucket) + except KeyError: + # This is probably a BucketWriter created by Foolscap, so just + # ignore it. + return uploads_index = self._uploads[storage_index] uploads_index.shares.pop(share_number) uploads_index.upload_secrets.pop(share_number) @@ -238,11 +252,15 @@ class _HTTPError(Exception): # Tags are of the form #6.nnn, where the number is documented at # https://www.iana.org/assignments/cbor-tags/cbor-tags.xhtml. Notably, #6.258 # indicates a set. +# +# Somewhat arbitrary limits are set to reduce e.g. number of shares, number of +# vectors, etc.. These may need to be iterated on in future revisions of the +# code. _SCHEMAS = { "allocate_buckets": Schema( """ request = { - share-numbers: #6.258([* uint]) + share-numbers: #6.258([0*256 uint]) allocated-size: uint } """ @@ -258,13 +276,13 @@ _SCHEMAS = { """ request = { "test-write-vectors": { - * share_number: { - "test": [* {"offset": uint, "size": uint, "specimen": bstr}] - "write": [* {"offset": uint, "data": bstr}] - "new-length": uint // null + 0*256 share_number : { + "test": [0*30 {"offset": uint, "size": uint, "specimen": bstr}] + "write": [0*30 {"offset": uint, "data": bstr}] + "new-length": uint / null } } - "read-vector": [* {"offset": uint, "size": uint}] + "read-vector": [0*30 {"offset": uint, "size": uint}] } share_number = uint """ @@ -272,6 +290,179 @@ _SCHEMAS = { } +# Callable that takes offset and length, returns the data at that range. +ReadData = Callable[[int, int], bytes] + + +@implementer(IPullProducer) +@define +class _ReadAllProducer: + """ + Producer that calls a read function repeatedly to read all the data, and + writes to a request. + """ + + request: Request + read_data: ReadData + result: Deferred = Factory(Deferred) + start: int = field(default=0) + + @classmethod + def produce_to(cls, request: Request, read_data: ReadData) -> Deferred: + """ + Create and register the producer, returning ``Deferred`` that should be + returned from a HTTP server endpoint. + """ + producer = cls(request, read_data) + request.registerProducer(producer, False) + return producer.result + + def resumeProducing(self): + data = self.read_data(self.start, 65536) + if not data: + self.request.unregisterProducer() + d = self.result + del self.result + d.callback(b"") + return + self.request.write(data) + self.start += len(data) + + def pauseProducing(self): + pass + + def stopProducing(self): + pass + + +@implementer(IPullProducer) +@define +class _ReadRangeProducer: + """ + Producer that calls a read function to read a range of data, and writes to + a request. + """ + + request: Request + read_data: ReadData + result: Deferred + start: int + remaining: int + + def resumeProducing(self): + to_read = min(self.remaining, 65536) + data = self.read_data(self.start, to_read) + assert len(data) <= to_read + + if not data and self.remaining > 0: + d, self.result = self.result, None + d.errback( + ValueError( + f"Should be {self.remaining} bytes left, but we got an empty read" + ) + ) + self.stopProducing() + return + + if len(data) > self.remaining: + d, self.result = self.result, None + d.errback( + ValueError( + f"Should be {self.remaining} bytes left, but we got more than that ({len(data)})!" + ) + ) + self.stopProducing() + return + + self.start += len(data) + self.remaining -= len(data) + assert self.remaining >= 0 + + self.request.write(data) + + if self.remaining == 0: + self.stopProducing() + + def pauseProducing(self): + pass + + def stopProducing(self): + if self.request is not None: + self.request.unregisterProducer() + self.request = None + if self.result is not None: + d = self.result + self.result = None + d.callback(b"") + + +def read_range( + request: Request, read_data: ReadData, share_length: int +) -> Union[Deferred, bytes]: + """ + Read an optional ``Range`` header, reads data appropriately via the given + callable, writes the data to the request. + + Only parses a subset of ``Range`` headers that we support: must be set, + bytes only, only a single range, the end must be explicitly specified. + Raises a ``_HTTPError(http.REQUESTED_RANGE_NOT_SATISFIABLE)`` if parsing is + not possible or the header isn't set. + + Takes a function that will do the actual reading given the start offset and + a length to read. + + The resulting data is written to the request. + """ + + def read_data_with_error_handling(offset: int, length: int) -> bytes: + try: + return read_data(offset, length) + except _HTTPError as e: + request.setResponseCode(e.code) + # Empty read means we're done. + return b"" + + if request.getHeader("range") is None: + return _ReadAllProducer.produce_to(request, read_data_with_error_handling) + + range_header = parse_range_header(request.getHeader("range")) + if ( + range_header is None # failed to parse + or range_header.units != "bytes" + or len(range_header.ranges) > 1 # more than one range + or range_header.ranges[0][1] is None # range without end + ): + raise _HTTPError(http.REQUESTED_RANGE_NOT_SATISFIABLE) + + offset, end = range_header.ranges[0] + # If we're being ask to read beyond the length of the share, just read + # less: + end = min(end, share_length) + if offset >= end: + # Basically we'd need to return an empty body. However, the + # Content-Range header can't actually represent empty lengths... so + # (mis)use 204 response code to indicate that. + raise _HTTPError(http.NO_CONTENT) + + request.setResponseCode(http.PARTIAL_CONTENT) + + # Actual conversion from Python's exclusive ranges to inclusive ranges is + # handled by werkzeug. + request.setHeader( + "content-range", + ContentRange("bytes", offset, end).to_header(), + ) + + d = Deferred() + request.registerProducer( + _ReadRangeProducer( + request, read_data_with_error_handling, d, offset, end - offset + ), + False, + ) + return d + + class HTTPServer(object): """ A HTTP interface to the storage server. @@ -323,9 +514,14 @@ class HTTPServer(object): accept = parse_accept_header(accept_headers[0]) if accept.best == CBOR_MIME_TYPE: request.setHeader("Content-Type", CBOR_MIME_TYPE) - # TODO if data is big, maybe want to use a temporary file eventually... - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3872 - return dumps(data) + f = TemporaryFile() + dump(data, f) + + def read_data(offset: int, length: int) -> bytes: + f.seek(offset) + return f.read(length) + + return _ReadAllProducer.produce_to(request, read_data) else: # TODO Might want to optionally send JSON someday: # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3861 @@ -334,12 +530,18 @@ class HTTPServer(object): def _read_encoded(self, request, schema: Schema) -> Any: """ Read encoded request body data, decoding it with CBOR by default. + + Somewhat arbitrarily, limit body size to 1MB; this may be too low, we + may want to customize per query type, but this is the starting point + for now. """ content_type = get_content_type(request.requestHeaders) if content_type == CBOR_MIME_TYPE: - # TODO limit memory usage, client could send arbitrarily large data... - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3872 - message = request.content.read() + # Read 1 byte more than 1MB. We expect length to be 1MB or + # less; if it's more assume it's not a legitimate message. + message = request.content.read(1024 * 1024 + 1) + if len(message) > 1024 * 1024: + raise _HTTPError(http.REQUEST_ENTITY_TOO_LARGE) schema.validate_cbor(message) result = loads(message) return result @@ -348,7 +550,7 @@ class HTTPServer(object): ##### Generic APIs ##### - @_authorized_route(_app, set(), "/v1/version", methods=["GET"]) + @_authorized_route(_app, set(), "/storage/v1/version", methods=["GET"]) def version(self, request, authorization): """Return version information.""" return self._send_encoded(request, self._storage_server.get_version()) @@ -358,7 +560,7 @@ class HTTPServer(object): @_authorized_route( _app, {Secrets.LEASE_RENEW, Secrets.LEASE_CANCEL, Secrets.UPLOAD}, - "/v1/immutable/", + "/storage/v1/immutable/", methods=["POST"], ) def allocate_buckets(self, request, authorization, storage_index): @@ -388,16 +590,13 @@ class HTTPServer(object): return self._send_encoded( request, - { - "already-have": set(already_got), - "allocated": set(sharenum_to_bucket), - }, + {"already-have": set(already_got), "allocated": set(sharenum_to_bucket)}, ) @_authorized_route( _app, {Secrets.UPLOAD}, - "/v1/immutable///abort", + "/storage/v1/immutable///abort", methods=["PUT"], ) def abort_share_upload(self, request, authorization, storage_index, share_number): @@ -428,7 +627,7 @@ class HTTPServer(object): @_authorized_route( _app, {Secrets.UPLOAD}, - "/v1/immutable//", + "/storage/v1/immutable//", methods=["PATCH"], ) def write_share_data(self, request, authorization, storage_index, share_number): @@ -438,20 +637,24 @@ class HTTPServer(object): request.setResponseCode(http.REQUESTED_RANGE_NOT_SATISFIABLE) return b"" - offset = content_range.start - - # TODO limit memory usage - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3872 - data = request.content.read(content_range.stop - content_range.start + 1) bucket = self._uploads.get_write_bucket( storage_index, share_number, authorization[Secrets.UPLOAD] ) + offset = content_range.start + remaining = content_range.stop - content_range.start + finished = False - try: - finished = bucket.write(offset, data) - except ConflictingWriteError: - request.setResponseCode(http.CONFLICT) - return b"" + while remaining > 0: + data = request.content.read(min(remaining, 65536)) + assert data, "uploaded data length doesn't match range" + + try: + finished = bucket.write(offset, data) + except ConflictingWriteError: + request.setResponseCode(http.CONFLICT) + return b"" + remaining -= len(data) + offset += len(data) if finished: bucket.close() @@ -467,7 +670,7 @@ class HTTPServer(object): @_authorized_route( _app, set(), - "/v1/immutable//shares", + "/storage/v1/immutable//shares", methods=["GET"], ) def list_shares(self, request, authorization, storage_index): @@ -480,7 +683,7 @@ class HTTPServer(object): @_authorized_route( _app, set(), - "/v1/immutable//", + "/storage/v1/immutable//", methods=["GET"], ) def read_share_chunk(self, request, authorization, storage_index, share_number): @@ -491,54 +694,17 @@ class HTTPServer(object): request.setResponseCode(http.NOT_FOUND) return b"" - if request.getHeader("range") is None: - # Return the whole thing. - start = 0 - while True: - # TODO should probably yield to event loop occasionally... - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3872 - data = bucket.read(start, start + 65536) - if not data: - request.finish() - return - request.write(data) - start += len(data) - - range_header = parse_range_header(request.getHeader("range")) - if ( - range_header is None - or range_header.units != "bytes" - or len(range_header.ranges) > 1 # more than one range - or range_header.ranges[0][1] is None # range without end - ): - request.setResponseCode(http.REQUESTED_RANGE_NOT_SATISFIABLE) - return b"" - - offset, end = range_header.ranges[0] - - # TODO limit memory usage - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3872 - data = bucket.read(offset, end - offset) - - request.setResponseCode(http.PARTIAL_CONTENT) - if len(data): - # For empty bodies the content-range header makes no sense since - # the end of the range is inclusive. - request.setHeader( - "content-range", - ContentRange("bytes", offset, offset + len(data)).to_header(), - ) - return data + return read_range(request, bucket.read, bucket.get_length()) @_authorized_route( _app, {Secrets.LEASE_RENEW, Secrets.LEASE_CANCEL}, - "/v1/lease/", + "/storage/v1/lease/", methods=["PUT"], ) def add_or_renew_lease(self, request, authorization, storage_index): - """Update the lease for an immutable share.""" - if not self._storage_server.get_buckets(storage_index): + """Update the lease for an immutable or mutable share.""" + if not list(self._storage_server.get_shares(storage_index)): raise _HTTPError(http.NOT_FOUND) # Checking of the renewal secret is done by the backend. @@ -554,7 +720,7 @@ class HTTPServer(object): @_authorized_route( _app, set(), - "/v1/immutable///corrupt", + "/storage/v1/immutable///corrupt", methods=["POST"], ) def advise_corrupt_share_immutable( @@ -575,79 +741,99 @@ class HTTPServer(object): @_authorized_route( _app, {Secrets.LEASE_RENEW, Secrets.LEASE_CANCEL, Secrets.WRITE_ENABLER}, - "/v1/mutable//read-test-write", + "/storage/v1/mutable//read-test-write", methods=["POST"], ) def mutable_read_test_write(self, request, authorization, storage_index): """Read/test/write combined operation for mutables.""" - # TODO unit tests rtw_request = self._read_encoded(request, _SCHEMAS["mutable_read_test_write"]) secrets = ( authorization[Secrets.WRITE_ENABLER], authorization[Secrets.LEASE_RENEW], authorization[Secrets.LEASE_CANCEL], ) - success, read_data = self._storage_server.slot_testv_and_readv_and_writev( - storage_index, - secrets, - { - k: ( - [(d["offset"], d["size"], b"eq", d["specimen"]) for d in v["test"]], - [(d["offset"], d["data"]) for d in v["write"]], - v["new-length"], - ) - for (k, v) in rtw_request["test-write-vectors"].items() - }, - [(d["offset"], d["size"]) for d in rtw_request["read-vector"]], - ) + try: + success, read_data = self._storage_server.slot_testv_and_readv_and_writev( + storage_index, + secrets, + { + k: ( + [ + (d["offset"], d["size"], b"eq", d["specimen"]) + for d in v["test"] + ], + [(d["offset"], d["data"]) for d in v["write"]], + v["new-length"], + ) + for (k, v) in rtw_request["test-write-vectors"].items() + }, + [(d["offset"], d["size"]) for d in rtw_request["read-vector"]], + ) + except BadWriteEnablerError: + raise _HTTPError(http.UNAUTHORIZED) return self._send_encoded(request, {"success": success, "data": read_data}) @_authorized_route( _app, set(), - "/v1/mutable//", + "/storage/v1/mutable//", methods=["GET"], ) def read_mutable_chunk(self, request, authorization, storage_index, share_number): """Read a chunk from a mutable.""" - if request.getHeader("range") is None: - # TODO in follow-up ticket - raise NotImplementedError() - # TODO reduce duplication with immutable reads? - # TODO unit tests, perhaps shared if possible - range_header = parse_range_header(request.getHeader("range")) - if ( - range_header is None - or range_header.units != "bytes" - or len(range_header.ranges) > 1 # more than one range - or range_header.ranges[0][1] is None # range without end - ): - request.setResponseCode(http.REQUESTED_RANGE_NOT_SATISFIABLE) - return b"" - - offset, end = range_header.ranges[0] - - # TODO limit memory usage - # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3872 - data = self._storage_server.slot_readv( - storage_index, [share_number], [(offset, end - offset)] - )[share_number][0] - - # TODO reduce duplication? - request.setResponseCode(http.PARTIAL_CONTENT) - if len(data): - # For empty bodies the content-range header makes no sense since - # the end of the range is inclusive. - request.setHeader( - "content-range", - ContentRange("bytes", offset, offset + len(data)).to_header(), + try: + share_length = self._storage_server.get_mutable_share_length( + storage_index, share_number ) - return data + except KeyError: + raise _HTTPError(http.NOT_FOUND) + + def read_data(offset, length): + try: + return self._storage_server.slot_readv( + storage_index, [share_number], [(offset, length)] + )[share_number][0] + except KeyError: + raise _HTTPError(http.NOT_FOUND) + + return read_range(request, read_data, share_length) + + @_authorized_route( + _app, + set(), + "/storage/v1/mutable//shares", + methods=["GET"], + ) + def enumerate_mutable_shares(self, request, authorization, storage_index): + """List mutable shares for a storage index.""" + shares = self._storage_server.enumerate_mutable_shares(storage_index) + return self._send_encoded(request, shares) + + @_authorized_route( + _app, + set(), + "/storage/v1/mutable///corrupt", + methods=["POST"], + ) + def advise_corrupt_share_mutable( + self, request, authorization, storage_index, share_number + ): + """Indicate that given share is corrupt, with a text reason.""" + if share_number not in { + shnum for (shnum, _) in self._storage_server.get_shares(storage_index) + }: + raise _HTTPError(http.NOT_FOUND) + + info = self._read_encoded(request, _SCHEMAS["advise_corrupt_share"]) + self._storage_server.advise_corrupt_share( + b"mutable", storage_index, share_number, info["reason"].encode("utf-8") + ) + return b"" @implementer(IStreamServerEndpoint) -@attr.s +@define class _TLSEndpointWrapper(object): """ Wrap an existing endpoint with the server-side storage TLS policy. This is @@ -655,8 +841,8 @@ class _TLSEndpointWrapper(object): example there's Tor and i2p. """ - endpoint = attr.ib(type=IStreamServerEndpoint) - context_factory = attr.ib(type=CertificateOptions) + endpoint: IStreamServerEndpoint + context_factory: CertificateOptions @classmethod def from_paths( @@ -681,6 +867,29 @@ class _TLSEndpointWrapper(object): ) +def build_nurl( + hostname: str, port: int, swissnum: str, certificate: CryptoCertificate +) -> DecodedURL: + """ + Construct a HTTPS NURL, given the hostname, port, server swissnum, and x509 + certificate for the server. Clients can then connect to the server using + this NURL. + """ + return DecodedURL().replace( + fragment="v=1", # how we know this NURL is HTTP-based (i.e. not Foolscap) + host=hostname, + port=port, + path=(swissnum,), + userinfo=( + str( + get_spki_hash(certificate), + "ascii", + ), + ), + scheme="pb", + ) + + def listen_tls( server: HTTPServer, hostname: str, @@ -700,22 +909,15 @@ def listen_tls( """ endpoint = _TLSEndpointWrapper.from_paths(endpoint, private_key_path, cert_path) - def build_nurl(listening_port: IListeningPort) -> DecodedURL: - nurl = DecodedURL().replace( - fragment="v=1", # how we know this NURL is HTTP-based (i.e. not Foolscap) - host=hostname, - port=listening_port.getHost().port, - path=(str(server._swissnum, "ascii"),), - userinfo=( - str( - get_spki_hash(load_pem_x509_certificate(cert_path.getContent())), - "ascii", - ), - ), - scheme="pb", + def get_nurl(listening_port: IListeningPort) -> DecodedURL: + address = cast(Union[IPv4Address, IPv6Address], listening_port.getHost()) + return build_nurl( + hostname, + address.port, + str(server._swissnum, "ascii"), + load_pem_x509_certificate(cert_path.getContent()), ) - return nurl return endpoint.listen(Site(server.get_resource())).addCallback( - lambda listening_port: (build_nurl(listening_port), listening_port) + lambda listening_port: (get_nurl(listening_port), listening_port) ) diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py index 920bd3c5e..0893513ae 100644 --- a/src/allmydata/storage/immutable.py +++ b/src/allmydata/storage/immutable.py @@ -199,8 +199,16 @@ class ShareFile(object): raise UnknownImmutableContainerVersionError(filename, version) self._num_leases = num_leases self._lease_offset = filesize - (num_leases * self.LEASE_SIZE) + self._length = filesize - 0xc - (num_leases * self.LEASE_SIZE) + self._data_offset = 0xc + def get_length(self): + """ + Return the length of the data in the share, if we're reading. + """ + return self._length + def unlink(self): os.unlink(self.home) @@ -389,7 +397,9 @@ class BucketWriter(object): """ Write data at given offset, return whether the upload is complete. """ - # Delay the timeout, since we received data: + # Delay the timeout, since we received data; if we get an + # AlreadyCancelled error, that means there's a bug in the client and + # write() was called after close(). self._timeout.reset(30 * 60) start = self._clock.seconds() precondition(not self.closed) @@ -411,14 +421,18 @@ class BucketWriter(object): self._already_written.set(True, offset, end) self.ss.add_latency("write", self._clock.seconds() - start) self.ss.count("write") + return self._is_finished() - # Return whether the whole thing has been written. See - # https://github.com/mlenzen/collections-extended/issues/169 and - # https://github.com/mlenzen/collections-extended/issues/172 for why - # it's done this way. + def _is_finished(self): + """ + Return whether the whole thing has been written. + """ return sum([mr.stop - mr.start for mr in self._already_written.ranges()]) == self._max_size def close(self): + # This can't actually be enabled, because it's not backwards compatible + # with old Foolscap clients. + # assert self._is_finished() precondition(not self.closed) self._timeout.cancel() start = self._clock.seconds() @@ -544,6 +558,12 @@ class BucketReader(object): self.shnum, reason) + def get_length(self): + """ + Return the length of the data in the share. + """ + return self._share_file.get_length() + @implementer(RIBucketReader) class FoolscapBucketReader(Referenceable): # type: ignore # warner/foolscap#78 diff --git a/src/allmydata/storage/mutable.py b/src/allmydata/storage/mutable.py index bd59d96b8..51c3a3c8b 100644 --- a/src/allmydata/storage/mutable.py +++ b/src/allmydata/storage/mutable.py @@ -412,11 +412,14 @@ class MutableShareFile(object): datav.append(self._read_share_data(f, offset, length)) return datav -# def remote_get_length(self): -# f = open(self.home, 'rb') -# data_length = self._read_data_length(f) -# f.close() -# return data_length + def get_length(self): + """ + Return the length of the data in the share. + """ + f = open(self.home, 'rb') + data_length = self._read_data_length(f) + f.close() + return data_length def check_write_enabler(self, write_enabler, si_s): with open(self.home, 'rb+') as f: diff --git a/src/allmydata/storage/server.py b/src/allmydata/storage/server.py index 9d1a3d6a4..2bf99d74c 100644 --- a/src/allmydata/storage/server.py +++ b/src/allmydata/storage/server.py @@ -1,18 +1,9 @@ """ Ported to Python 3. """ -from __future__ import division -from __future__ import absolute_import -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import bytes_to_native_str, PY2 -if PY2: - # Omit open() to get native behavior where open("w") always accepts native - # strings. Omit bytes so we don't leak future's custom bytes. - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, pow, round, super, dict, list, object, range, str, max, min # noqa: F401 -else: - from typing import Dict, Tuple +from __future__ import annotations +from future.utils import bytes_to_native_str +from typing import Dict, Tuple, Iterable import os, re @@ -330,7 +321,7 @@ class StorageServer(service.MultiService): # they asked about: this will save them a lot of work. Add or update # leases for all of them: if they want us to hold shares for this # file, they'll want us to hold leases for this file. - for (shnum, fn) in self._get_bucket_shares(storage_index): + for (shnum, fn) in self.get_shares(storage_index): alreadygot[shnum] = ShareFile(fn) if renew_leases: self._add_or_renew_leases(alreadygot.values(), lease_info) @@ -372,7 +363,7 @@ class StorageServer(service.MultiService): return set(alreadygot), bucketwriters def _iter_share_files(self, storage_index): - for shnum, filename in self._get_bucket_shares(storage_index): + for shnum, filename in self.get_shares(storage_index): with open(filename, 'rb') as f: header = f.read(32) if MutableShareFile.is_valid_header(header): @@ -425,10 +416,12 @@ class StorageServer(service.MultiService): """ self._call_on_bucket_writer_close.append(handler) - def _get_bucket_shares(self, storage_index): - """Return a list of (shnum, pathname) tuples for files that hold + def get_shares(self, storage_index) -> Iterable[tuple[int, str]]: + """ + Return an iterable of (shnum, pathname) tuples for files that hold shares for this storage_index. In each tuple, 'shnum' will always be - the integer form of the last component of 'pathname'.""" + the integer form of the last component of 'pathname'. + """ storagedir = os.path.join(self.sharedir, storage_index_to_dir(storage_index)) try: for f in os.listdir(storagedir): @@ -440,12 +433,15 @@ class StorageServer(service.MultiService): pass def get_buckets(self, storage_index): + """ + Get ``BucketReaders`` for an immutable. + """ start = self._clock.seconds() self.count("get") si_s = si_b2a(storage_index) log.msg("storage: get_buckets %r" % si_s) bucketreaders = {} # k: sharenum, v: BucketReader - for shnum, filename in self._get_bucket_shares(storage_index): + for shnum, filename in self.get_shares(storage_index): bucketreaders[shnum] = BucketReader(self, filename, storage_index, shnum) self.add_latency("get", self._clock.seconds() - start) @@ -462,7 +458,7 @@ class StorageServer(service.MultiService): # since all shares get the same lease data, we just grab the leases # from the first share try: - shnum, filename = next(self._get_bucket_shares(storage_index)) + shnum, filename = next(self.get_shares(storage_index)) sf = ShareFile(filename) return sf.get_leases() except StopIteration: @@ -476,7 +472,7 @@ class StorageServer(service.MultiService): :return: An iterable of the leases attached to this slot. """ - for _, share_filename in self._get_bucket_shares(storage_index): + for _, share_filename in self.get_shares(storage_index): share = MutableShareFile(share_filename) return share.get_leases() return [] @@ -699,6 +695,21 @@ class StorageServer(service.MultiService): self) return share + def enumerate_mutable_shares(self, storage_index: bytes) -> set[int]: + """Return all share numbers for the given mutable.""" + si_dir = storage_index_to_dir(storage_index) + # shares exist if there is a file for them + bucketdir = os.path.join(self.sharedir, si_dir) + if not os.path.isdir(bucketdir): + return set() + result = set() + for sharenum_s in os.listdir(bucketdir): + try: + result.add(int(sharenum_s)) + except ValueError: + continue + return result + def slot_readv(self, storage_index, shares, readv): start = self._clock.seconds() self.count("readv") @@ -736,7 +747,7 @@ class StorageServer(service.MultiService): :return bool: ``True`` if a share with the given number exists at the given storage index, ``False`` otherwise. """ - for existing_sharenum, ignored in self._get_bucket_shares(storage_index): + for existing_sharenum, ignored in self.get_shares(storage_index): if existing_sharenum == shnum: return True return False @@ -783,6 +794,20 @@ class StorageServer(service.MultiService): return None + def get_immutable_share_length(self, storage_index: bytes, share_number: int) -> int: + """Returns the length (in bytes) of an immutable.""" + si_dir = storage_index_to_dir(storage_index) + path = os.path.join(self.sharedir, si_dir, str(share_number)) + return ShareFile(path).get_length() + + def get_mutable_share_length(self, storage_index: bytes, share_number: int) -> int: + """Returns the length (in bytes) of a mutable.""" + si_dir = storage_index_to_dir(storage_index) + path = os.path.join(self.sharedir, si_dir, str(share_number)) + if not os.path.exists(path): + raise KeyError("No such storage index or share number") + return MutableShareFile(path).get_length() + @implementer(RIStorageServer) class FoolscapStorageServer(Referenceable): # type: ignore # warner/foolscap#78 diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index a3974d4b9..410bfd28b 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -5,10 +5,6 @@ the foolscap-based server implemented in src/allmydata/storage/*.py . Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals # roadmap: # @@ -34,23 +30,25 @@ from __future__ import unicode_literals # # 6: implement other sorts of IStorageClient classes: S3, etc -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 -from six import ensure_text +from __future__ import annotations +from six import ensure_text +from typing import Union import re, time, hashlib from os import urandom -# On Python 2 this will be the backport. from configparser import NoSectionError import attr +from hyperlink import DecodedURL from zope.interface import ( Attribute, Interface, implementer, ) -from twisted.internet import defer +from twisted.python.failure import Failure +from twisted.web import http +from twisted.internet.task import LoopingCall +from twisted.internet import defer, reactor from twisted.application import service from twisted.logger import Logger from twisted.plugin import ( @@ -76,12 +74,15 @@ from allmydata.util.observer import ObserverList from allmydata.util.rrefutil import add_version_to_remote_reference from allmydata.util.hashutil import permute_server_hash from allmydata.util.dictutil import BytesKeyDict, UnicodeKeyDict +from allmydata.util.deferredutil import async_to_deferred from allmydata.storage.http_client import ( StorageClient, StorageClientImmutables, StorageClientGeneral, ClientException as HTTPClientException, StorageClientMutables, - ReadVector, TestWriteVectors, WriteVector, TestVector + ReadVector, TestWriteVectors, WriteVector, TestVector, ClientException ) +ANONYMOUS_STORAGE_NURLS = "anonymous-storage-NURLs" + # who is responsible for de-duplication? # both? @@ -106,8 +107,8 @@ class StorageClientConfig(object): :ivar preferred_peers: An iterable of the server-ids (``bytes``) of the storage servers where share placement is preferred, in order of - decreasing preference. See the *[client]peers.preferred* - documentation for details. + decreasing preference. See the *[client]peers.preferred* documentation + for details. :ivar dict[unicode, dict[unicode, unicode]] storage_plugins: A mapping from names of ``IFoolscapStoragePlugin`` configured in *tahoe.cfg* to the @@ -269,6 +270,10 @@ class StorageFarmBroker(service.MultiService): by the given announcement. """ assert isinstance(server_id, bytes) + if len(server["ann"].get(ANONYMOUS_STORAGE_NURLS, [])) > 0: + s = HTTPNativeStorageServer(server_id, server["ann"]) + s.on_status_changed(lambda _: self._got_connection()) + return s handler_overrides = server.get("connections", {}) s = NativeStorageServer( server_id, @@ -530,6 +535,45 @@ class IFoolscapStorageServer(Interface): """ +def _parse_announcement(server_id: bytes, furl: bytes, ann: dict) -> tuple[str, bytes, bytes, bytes, bytes]: + """ + Parse the furl and announcement, return: + + (nickname, permutation_seed, tubid, short_description, long_description) + """ + m = re.match(br'pb://(\w+)@', furl) + assert m, furl + tubid_s = m.group(1).lower() + tubid = base32.a2b(tubid_s) + if "permutation-seed-base32" in ann: + seed = ann["permutation-seed-base32"] + if isinstance(seed, str): + seed = seed.encode("utf-8") + ps = base32.a2b(seed) + elif re.search(br'^v0-[0-9a-zA-Z]{52}$', server_id): + ps = base32.a2b(server_id[3:]) + else: + log.msg("unable to parse serverid '%(server_id)s as pubkey, " + "hashing it to get permutation-seed, " + "may not converge with other clients", + server_id=server_id, + facility="tahoe.storage_broker", + level=log.UNUSUAL, umid="qu86tw") + ps = hashlib.sha256(server_id).digest() + permutation_seed = ps + + assert server_id + long_description = server_id + if server_id.startswith(b"v0-"): + # remove v0- prefix from abbreviated name + short_description = server_id[3:3+8] + else: + short_description = server_id[:8] + nickname = ann.get("nickname", "") + + return (nickname, permutation_seed, tubid, short_description, long_description) + + @implementer(IFoolscapStorageServer) @attr.s(frozen=True) class _FoolscapStorage(object): @@ -573,43 +617,13 @@ class _FoolscapStorage(object): The furl will be a Unicode string on Python 3; on Python 2 it will be either a native (bytes) string or a Unicode string. """ - furl = furl.encode("utf-8") - m = re.match(br'pb://(\w+)@', furl) - assert m, furl - tubid_s = m.group(1).lower() - tubid = base32.a2b(tubid_s) - if "permutation-seed-base32" in ann: - seed = ann["permutation-seed-base32"] - if isinstance(seed, str): - seed = seed.encode("utf-8") - ps = base32.a2b(seed) - elif re.search(br'^v0-[0-9a-zA-Z]{52}$', server_id): - ps = base32.a2b(server_id[3:]) - else: - log.msg("unable to parse serverid '%(server_id)s as pubkey, " - "hashing it to get permutation-seed, " - "may not converge with other clients", - server_id=server_id, - facility="tahoe.storage_broker", - level=log.UNUSUAL, umid="qu86tw") - ps = hashlib.sha256(server_id).digest() - permutation_seed = ps - - assert server_id - long_description = server_id - if server_id.startswith(b"v0-"): - # remove v0- prefix from abbreviated name - short_description = server_id[3:3+8] - else: - short_description = server_id[:8] - nickname = ann.get("nickname", "") - + (nickname, permutation_seed, tubid, short_description, long_description) = _parse_announcement(server_id, furl.encode("utf-8"), ann) return cls( nickname=nickname, permutation_seed=permutation_seed, tubid=tubid, storage_server=storage_server, - furl=furl, + furl=furl.encode("utf-8"), short_description=short_description, long_description=long_description, ) @@ -708,6 +722,16 @@ def _storage_from_foolscap_plugin(node_config, config, announcement, get_rref): raise AnnouncementNotMatched(plugin_names) +def _available_space_from_version(version): + if version is None: + return None + protocol_v1_version = version.get(b'http://allmydata.org/tahoe/protocols/storage/v1', BytesKeyDict()) + available_space = protocol_v1_version.get(b'available-space') + if available_space is None: + available_space = protocol_v1_version.get(b'maximum-immutable-share-size', None) + return available_space + + @implementer(IServer) class NativeStorageServer(service.MultiService): """I hold information about a storage server that we want to connect to. @@ -875,13 +899,7 @@ class NativeStorageServer(service.MultiService): def get_available_space(self): version = self.get_version() - if version is None: - return None - protocol_v1_version = version.get(b'http://allmydata.org/tahoe/protocols/storage/v1', BytesKeyDict()) - available_space = protocol_v1_version.get(b'available-space') - if available_space is None: - available_space = protocol_v1_version.get(b'maximum-immutable-share-size', None) - return available_space + return _available_space_from_version(version) def start_connecting(self, trigger_cb): self._tub = self._tub_maker(self._handler_overrides) @@ -943,6 +961,164 @@ class NativeStorageServer(service.MultiService): # used when the broker wants us to hurry up self._reconnector.reset() + +@implementer(IServer) +class HTTPNativeStorageServer(service.MultiService): + """ + Like ``NativeStorageServer``, but for HTTP clients. + + The notion of being "connected" is less meaningful for HTTP; we just poll + occasionally, and if we've succeeded at last poll, we assume we're + "connected". + """ + + def __init__(self, server_id: bytes, announcement, reactor=reactor): + service.MultiService.__init__(self) + assert isinstance(server_id, bytes) + self._server_id = server_id + self.announcement = announcement + self._on_status_changed = ObserverList() + self._reactor = reactor + furl = announcement["anonymous-storage-FURL"].encode("utf-8") + ( + self._nickname, + self._permutation_seed, + self._tubid, + self._short_description, + self._long_description + ) = _parse_announcement(server_id, furl, announcement) + # TODO need some way to do equivalent of Happy Eyeballs for multiple NURLs? + # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3935 + nurl = DecodedURL.from_text(announcement[ANONYMOUS_STORAGE_NURLS][0]) + self._istorage_server = _HTTPStorageServer.from_http_client( + StorageClient.from_nurl(nurl, reactor) + ) + + self._connection_status = connection_status.ConnectionStatus.unstarted() + self._version = None + self._last_connect_time = None + self._connecting_deferred = None + + def get_permutation_seed(self): + return self._permutation_seed + + def get_name(self): + return self._short_description + + def get_longname(self): + return self._long_description + + def get_tubid(self): + return self._tubid + + def get_lease_seed(self): + # Apparently this is what Foolscap version above does?! + return self._tubid + + def get_foolscap_write_enabler_seed(self): + return self._tubid + + def get_nickname(self): + return self._nickname + + def on_status_changed(self, status_changed): + """ + :param status_changed: a callable taking a single arg (the + NativeStorageServer) that is notified when we become connected + """ + return self._on_status_changed.subscribe(status_changed) + + # Special methods used by copy.copy() and copy.deepcopy(). When those are + # used in allmydata.immutable.filenode to copy CheckResults during + # repair, we want it to treat the IServer instances as singletons, and + # not attempt to duplicate them.. + def __copy__(self): + return self + + def __deepcopy__(self, memodict): + return self + + def __repr__(self): + return "" % self.get_name() + + def get_serverid(self): + return self._server_id + + def get_version(self): + return self._version + + def get_announcement(self): + return self.announcement + + def get_connection_status(self): + return self._connection_status + + def is_connected(self): + return self._connection_status.connected + + def get_available_space(self): + version = self.get_version() + return _available_space_from_version(version) + + def start_connecting(self, trigger_cb): + self._lc = LoopingCall(self._connect) + self._lc.start(1, True) + + def _got_version(self, version): + self._last_connect_time = time.time() + self._version = version + self._connection_status = connection_status.ConnectionStatus( + True, "connected", [], self._last_connect_time, self._last_connect_time + ) + self._on_status_changed.notify(self) + + def _failed_to_connect(self, reason): + self._connection_status = connection_status.ConnectionStatus( + False, f"failure: {reason}", [], self._last_connect_time, self._last_connect_time + ) + self._on_status_changed.notify(self) + + def get_storage_server(self): + """ + See ``IServer.get_storage_server``. + """ + if self._connection_status.summary == "unstarted": + return None + return self._istorage_server + + def stop_connecting(self): + self._lc.stop() + if self._connecting_deferred is not None: + self._connecting_deferred.cancel() + + def try_to_connect(self): + self._connect() + + def _connect(self): + result = self._istorage_server.get_version() + + def remove_connecting_deferred(result): + self._connecting_deferred = None + return result + + # Set a short timeout since we're relying on this for server liveness. + self._connecting_deferred = result.addTimeout(5, self._reactor).addBoth( + remove_connecting_deferred).addCallbacks( + self._got_version, + self._failed_to_connect + ) + + def stopService(self): + if self._connecting_deferred is not None: + self._connecting_deferred.cancel() + + result = service.MultiService.stopService(self) + if self._lc.running: + self._lc.stop() + self._failed_to_connect("shut down") + return result + + class UnknownServerTypeError(Exception): pass @@ -1059,7 +1235,7 @@ class _StorageServer(object): -@attr.s +@attr.s(hash=True) class _FakeRemoteReference(object): """ Emulate a Foolscap RemoteReference, calling a local object instead. @@ -1084,7 +1260,7 @@ class _HTTPBucketWriter(object): storage_index = attr.ib(type=bytes) share_number = attr.ib(type=int) upload_secret = attr.ib(type=bytes) - finished = attr.ib(type=bool, default=False) + finished = attr.ib(type=defer.Deferred[bool], factory=defer.Deferred) def abort(self): return self.client.abort_upload(self.storage_index, self.share_number, @@ -1096,18 +1272,27 @@ class _HTTPBucketWriter(object): self.storage_index, self.share_number, self.upload_secret, offset, data ) if result.finished: - self.finished = True + self.finished.callback(True) defer.returnValue(None) def close(self): - # A no-op in HTTP protocol. - if not self.finished: - return defer.fail(RuntimeError("You didn't finish writing?!")) - return defer.succeed(None) + # We're not _really_ closed until all writes have succeeded and we + # finished writing all the data. + return self.finished +def _ignore_404(failure: Failure) -> Union[Failure, None]: + """ + Useful for advise_corrupt_share(), since it swallows unknown share numbers + in Foolscap. + """ + if failure.check(HTTPClientException) and failure.value.code == http.NOT_FOUND: + return None + else: + return failure -@attr.s + +@attr.s(hash=True) class _HTTPBucketReader(object): """ Emulate a ``RIBucketReader``, but use HTTP protocol underneath. @@ -1125,7 +1310,7 @@ class _HTTPBucketReader(object): return self.client.advise_corrupt_share( self.storage_index, self.share_number, str(reason, "utf-8", errors="backslashreplace") - ) + ).addErrback(_ignore_404) # WORK IN PROGRESS, for now it doesn't actually implement whole thing. @@ -1192,16 +1377,23 @@ class _HTTPStorageServer(object): for share_num in share_numbers }) - def add_lease( + @async_to_deferred + async def add_lease( self, storage_index, renew_secret, cancel_secret ): - immutable_client = StorageClientImmutables(self._http_client) - return immutable_client.add_or_renew_lease( - storage_index, renew_secret, cancel_secret - ) + client = StorageClientGeneral(self._http_client) + try: + await client.add_or_renew_lease( + storage_index, renew_secret, cancel_secret + ) + except ClientException as e: + if e.code == http.NOT_FOUND: + # Silently do nothing, as is the case for the Foolscap client + return + raise def advise_corrupt_share( self, @@ -1211,21 +1403,24 @@ class _HTTPStorageServer(object): reason: bytes ): if share_type == b"immutable": - imm_client = StorageClientImmutables(self._http_client) - return imm_client.advise_corrupt_share( - storage_index, shnum, str(reason, "utf-8", errors="backslashreplace") - ) + client : Union[StorageClientImmutables, StorageClientMutables] = StorageClientImmutables(self._http_client) + elif share_type == b"mutable": + client = StorageClientMutables(self._http_client) else: - raise NotImplementedError() # future tickets + raise ValueError("Unknown share type") + return client.advise_corrupt_share( + storage_index, shnum, str(reason, "utf-8", errors="backslashreplace") + ).addErrback(_ignore_404) @defer.inlineCallbacks def slot_readv(self, storage_index, shares, readv): mutable_client = StorageClientMutables(self._http_client) pending_reads = {} reads = {} - # TODO if shares list is empty, that means list all shares, so we need + # If shares list is empty, that means list all shares, so we need # to do a query to get that. - assert shares # TODO replace with call to list shares if and only if it's empty + if not shares: + shares = yield mutable_client.list_shares(storage_index) # Start all the queries in parallel: for share_number in shares: @@ -1273,8 +1468,13 @@ class _HTTPStorageServer(object): ReadVector(offset=offset, size=size) for (offset, size) in r_vector ] - client_result = yield mutable_client.read_test_write_chunks( - storage_index, we_secret, lr_secret, lc_secret, client_tw_vectors, - client_read_vectors, - ) + try: + client_result = yield mutable_client.read_test_write_chunks( + storage_index, we_secret, lr_secret, lc_secret, client_tw_vectors, + client_read_vectors, + ) + except ClientException as e: + if e.code == http.UNAUTHORIZED: + raise RemoteException("Unauthorized write, possibly you passed the wrong write enabler?") + raise return (client_result.success, client_result.reads) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index 28613e8c1..e84f52096 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -12,23 +12,19 @@ from future.utils import PY2 if PY2: from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +import re from six.moves import ( StringIO, ) -from testtools import ( - skipIf, -) +from hypothesis.strategies import text +from hypothesis import given, assume from testtools.matchers import ( Contains, Equals, - HasLength, ) -from twisted.python.runtime import ( - platform, -) from twisted.python.filepath import ( FilePath, ) @@ -44,6 +40,10 @@ from ...scripts.tahoe_run import ( RunOptions, run, ) +from ...util.pid import ( + check_pid_process, + InvalidPidFile, +) from ...scripts.runner import ( parse_options @@ -151,7 +151,7 @@ class RunTests(SyncTestCase): """ Tests for ``run``. """ - @skipIf(platform.isWindows(), "There are no PID files on Windows.") + def test_non_numeric_pid(self): """ If the pidfile exists but does not contain a numeric value, a complaint to @@ -159,7 +159,7 @@ class RunTests(SyncTestCase): """ basedir = FilePath(self.mktemp()).asTextMode() basedir.makedirs() - basedir.child(u"twistd.pid").setContent(b"foo") + basedir.child(u"running.process").setContent(b"foo") basedir.child(u"tahoe-client.tac").setContent(b"") config = RunOptions() @@ -168,17 +168,30 @@ class RunTests(SyncTestCase): config['basedir'] = basedir.path config.twistd_args = [] + reactor = MemoryReactor() + runs = [] - result_code = run(config, runApp=runs.append) + result_code = run(reactor, config, runApp=runs.append) self.assertThat( config.stderr.getvalue(), Contains("found invalid PID file in"), ) - self.assertThat( - runs, - HasLength(1), - ) - self.assertThat( - result_code, - Equals(0), - ) + # because the pidfile is invalid we shouldn't get to the + # .run() call itself. + self.assertThat(runs, Equals([])) + self.assertThat(result_code, Equals(1)) + + good_file_content_re = re.compile(r"\w[0-9]*\w[0-9]*\w") + + @given(text()) + def test_pidfile_contents(self, content): + """ + invalid contents for a pidfile raise errors + """ + assume(not self.good_file_content_re.match(content)) + pidfile = FilePath("pidfile") + pidfile.setContent(content.encode("utf8")) + + with self.assertRaises(InvalidPidFile): + with check_pid_process(pidfile): + pass diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py index 410796be2..c324d5565 100644 --- a/src/allmydata/test/cli_node_api.py +++ b/src/allmydata/test/cli_node_api.py @@ -134,7 +134,7 @@ class CLINodeAPI(object): @property def twistd_pid_file(self): - return self.basedir.child(u"twistd.pid") + return self.basedir.child(u"running.process") @property def node_url_file(self): diff --git a/src/allmydata/test/common_system.py b/src/allmydata/test/common_system.py index 9851d2b91..01966824a 100644 --- a/src/allmydata/test/common_system.py +++ b/src/allmydata/test/common_system.py @@ -5,22 +5,14 @@ in ``allmydata.test.test_system``. Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - # Don't import bytes since it causes issues on (so far unported) modules on Python 2. - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, dict, list, object, range, max, min, str # noqa: F401 - +from typing import Optional import os from functools import partial from twisted.internet import reactor from twisted.internet import defer from twisted.internet.defer import inlineCallbacks +from twisted.internet.task import deferLater from twisted.application import service from foolscap.api import flushEventualQueue @@ -28,6 +20,12 @@ from foolscap.api import flushEventualQueue from allmydata import client from allmydata.introducer.server import create_introducer from allmydata.util import fileutil, log, pollmixin +from allmydata.util.deferredutil import async_to_deferred +from allmydata.storage import http_client +from allmydata.storage_client import ( + NativeStorageServer, + HTTPNativeStorageServer, +) from twisted.python.filepath import ( FilePath, @@ -642,9 +640,51 @@ def _render_section_values(values): )) +@async_to_deferred +async def spin_until_cleanup_done(value=None, timeout=10): + """ + At the end of the test, spin until the reactor has no more DelayedCalls + and file descriptors (or equivalents) registered. This prevents dirty + reactor errors, while also not hard-coding a fixed amount of time, so it + can finish faster on faster computers. + + There is also a timeout: if it takes more than 10 seconds (by default) for + the remaining reactor state to clean itself up, the presumption is that it + will never get cleaned up and the spinning stops. + + Make sure to run as last thing in tearDown. + """ + def num_fds(): + if hasattr(reactor, "handles"): + # IOCP! + return len(reactor.handles) + else: + # Normal reactor; having internal readers still registered is fine, + # that's not our code. + return len( + set(reactor.getReaders()) - set(reactor._internalReaders) + ) + len(reactor.getWriters()) + + for i in range(timeout * 1000): + # There's a single DelayedCall for AsynchronousDeferredRunTest's + # timeout... + if (len(reactor.getDelayedCalls()) < 2 and num_fds() == 0): + break + await deferLater(reactor, 0.001) + return value + + class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin): + # If set to True, use Foolscap for storage protocol. If set to False, HTTP + # will be used when possible. If set to None, this suggests a bug in the + # test code. + FORCE_FOOLSCAP_FOR_STORAGE : Optional[bool] = None + def setUp(self): + self._http_client_pools = [] + http_client.StorageClient.start_test_mode(self._got_new_http_connection_pool) + self.addCleanup(http_client.StorageClient.stop_test_mode) self.port_assigner = SameProcessStreamEndpointAssigner() self.port_assigner.setUp() self.addCleanup(self.port_assigner.tearDown) @@ -652,10 +692,35 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin): self.sparent = service.MultiService() self.sparent.startService() + def _got_new_http_connection_pool(self, pool): + # Register the pool for shutdown later: + self._http_client_pools.append(pool) + # Disable retries: + pool.retryAutomatically = False + # Make a much more aggressive timeout for connections, we're connecting + # locally after all... and also make sure it's lower than the delay we + # add in tearDown, to prevent dirty reactor issues. + getConnection = pool.getConnection + + def getConnectionWithTimeout(*args, **kwargs): + d = getConnection(*args, **kwargs) + d.addTimeout(1, reactor) + return d + + pool.getConnection = getConnectionWithTimeout + + def close_idle_http_connections(self): + """Close all HTTP client connections that are just hanging around.""" + return defer.gatherResults( + [pool.closeCachedConnections() for pool in self._http_client_pools] + ) + def tearDown(self): log.msg("shutting down SystemTest services") d = self.sparent.stopService() d.addBoth(flush_but_dont_ignore) + d.addBoth(lambda x: self.close_idle_http_connections().addCallback(lambda _: x)) + d.addBoth(spin_until_cleanup_done) return d def getdir(self, subdir): @@ -714,21 +779,31 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin): :return: A ``Deferred`` that fires when the nodes have connected to each other. """ + self.assertIn( + self.FORCE_FOOLSCAP_FOR_STORAGE, (True, False), + "You forgot to set FORCE_FOOLSCAP_FOR_STORAGE on {}".format(self.__class__) + ) self.numclients = NUMCLIENTS self.introducer = yield self._create_introducer() self.add_service(self.introducer) self.introweb_url = self._get_introducer_web() - yield self._set_up_client_nodes() + yield self._set_up_client_nodes(self.FORCE_FOOLSCAP_FOR_STORAGE) + native_server = next(iter(self.clients[0].storage_broker.get_known_servers())) + if self.FORCE_FOOLSCAP_FOR_STORAGE: + expected_storage_server_class = NativeStorageServer + else: + expected_storage_server_class = HTTPNativeStorageServer + self.assertIsInstance(native_server, expected_storage_server_class) @inlineCallbacks - def _set_up_client_nodes(self): + def _set_up_client_nodes(self, force_foolscap): q = self.introducer self.introducer_furl = q.introducer_url self.clients = [] basedirs = [] for i in range(self.numclients): - basedirs.append((yield self._set_up_client_node(i))) + basedirs.append((yield self._set_up_client_node(i, force_foolscap))) # start clients[0], wait for it's tub to be ready (at which point it # will have registered the helper furl). @@ -761,7 +836,7 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin): # and the helper-using webport self.helper_webish_url = self.clients[3].getServiceNamed("webish").getURL() - def _generate_config(self, which, basedir): + def _generate_config(self, which, basedir, force_foolscap=False): config = {} allclients = set(range(self.numclients)) @@ -791,6 +866,7 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin): sethelper = partial(setconf, config, which, "helper") setnode("nickname", u"client %d \N{BLACK SMILING FACE}" % (which,)) + setconf(config, which, "storage", "force_foolscap", str(force_foolscap)) tub_location_hint, tub_port_endpoint = self.port_assigner.assign(reactor) setnode("tub.port", tub_port_endpoint) @@ -808,17 +884,16 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin): " furl: %s\n") % self.introducer_furl iyaml_fn = os.path.join(basedir, "private", "introducers.yaml") fileutil.write(iyaml_fn, iyaml) - return _render_config(config) - def _set_up_client_node(self, which): + def _set_up_client_node(self, which, force_foolscap): basedir = self.getdir("client%d" % (which,)) fileutil.make_dirs(os.path.join(basedir, "private")) if len(SYSTEM_TEST_CERTS) > (which + 1): f = open(os.path.join(basedir, "private", "node.pem"), "w") f.write(SYSTEM_TEST_CERTS[which + 1]) f.close() - config = self._generate_config(which, basedir) + config = self._generate_config(which, basedir, force_foolscap) fileutil.write(os.path.join(basedir, 'tahoe.cfg'), config) return basedir diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py index e63c3eef8..b6d352ab1 100644 --- a/src/allmydata/test/common_util.py +++ b/src/allmydata/test/common_util.py @@ -145,6 +145,7 @@ def run_cli_native(verb, *args, **kwargs): ) d.addCallback( runner.dispatch, + reactor, stdin=stdin, stdout=stdout, stderr=stderr, diff --git a/src/allmydata/test/test_consumer.py b/src/allmydata/test/test_consumer.py index a689de462..234fc2594 100644 --- a/src/allmydata/test/test_consumer.py +++ b/src/allmydata/test/test_consumer.py @@ -14,11 +14,17 @@ if PY2: from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 from zope.interface import implementer -from twisted.trial.unittest import TestCase from twisted.internet.interfaces import IPushProducer, IPullProducer from allmydata.util.consumer import MemoryConsumer +from .common import ( + SyncTestCase, +) +from testtools.matchers import ( + Equals, +) + @implementer(IPushProducer) @implementer(IPullProducer) @@ -50,7 +56,7 @@ class Producer(object): self.consumer.unregisterProducer() -class MemoryConsumerTests(TestCase): +class MemoryConsumerTests(SyncTestCase): """Tests for MemoryConsumer.""" def test_push_producer(self): @@ -60,14 +66,14 @@ class MemoryConsumerTests(TestCase): consumer = MemoryConsumer() producer = Producer(consumer, [b"abc", b"def", b"ghi"]) consumer.registerProducer(producer, True) - self.assertEqual(consumer.chunks, [b"abc"]) + self.assertThat(consumer.chunks, Equals([b"abc"])) producer.iterate() producer.iterate() - self.assertEqual(consumer.chunks, [b"abc", b"def", b"ghi"]) - self.assertEqual(consumer.done, False) + self.assertThat(consumer.chunks, Equals([b"abc", b"def", b"ghi"])) + self.assertFalse(consumer.done) producer.iterate() - self.assertEqual(consumer.chunks, [b"abc", b"def", b"ghi"]) - self.assertEqual(consumer.done, True) + self.assertThat(consumer.chunks, Equals([b"abc", b"def", b"ghi"])) + self.assertTrue(consumer.done) def test_pull_producer(self): """ @@ -76,8 +82,8 @@ class MemoryConsumerTests(TestCase): consumer = MemoryConsumer() producer = Producer(consumer, [b"abc", b"def", b"ghi"]) consumer.registerProducer(producer, False) - self.assertEqual(consumer.chunks, [b"abc", b"def", b"ghi"]) - self.assertEqual(consumer.done, True) + self.assertThat(consumer.chunks, Equals([b"abc", b"def", b"ghi"])) + self.assertTrue(consumer.done) # download_to_data() is effectively tested by some of the filenode tests, e.g. diff --git a/src/allmydata/test/test_istorageserver.py b/src/allmydata/test/test_istorageserver.py index e7b869713..9e7e7b6e1 100644 --- a/src/allmydata/test/test_istorageserver.py +++ b/src/allmydata/test/test_istorageserver.py @@ -15,26 +15,16 @@ from typing import Set from random import Random from unittest import SkipTest -from twisted.internet.defer import inlineCallbacks, returnValue, succeed +from twisted.internet.defer import inlineCallbacks, returnValue from twisted.internet.task import Clock -from twisted.internet import reactor -from twisted.internet.endpoints import serverFromString -from twisted.python.filepath import FilePath from foolscap.api import Referenceable, RemoteException -from allmydata.interfaces import IStorageServer # really, IStorageClient +# A better name for this would be IStorageClient... +from allmydata.interfaces import IStorageServer + from .common_system import SystemTestMixin -from .common import AsyncTestCase, SameProcessStreamEndpointAssigner -from .certs import ( - generate_certificate, - generate_private_key, - private_key_to_file, - cert_to_file, -) +from .common import AsyncTestCase from allmydata.storage.server import StorageServer # not a IStorageServer!! -from allmydata.storage.http_server import HTTPServer, listen_tls -from allmydata.storage.http_client import StorageClient -from allmydata.storage_client import _HTTPStorageServer # Use random generator with known seed, so results are reproducible if tests @@ -446,6 +436,17 @@ class IStorageServerImmutableAPIsTestsMixin(object): b"immutable", storage_index, 0, b"ono" ) + @inlineCallbacks + def test_advise_corrupt_share_unknown_share_number(self): + """ + Calling ``advise_corrupt_share()`` on an immutable share, with an + unknown share number, does not result in error. + """ + storage_index, _, _ = yield self.create_share() + yield self.storage_client.advise_corrupt_share( + b"immutable", storage_index, 999, b"ono" + ) + @inlineCallbacks def test_allocate_buckets_creates_lease(self): """ @@ -459,6 +460,21 @@ class IStorageServerImmutableAPIsTestsMixin(object): lease.get_expiration_time() - self.fake_time() > (31 * 24 * 60 * 60 - 10) ) + @inlineCallbacks + def test_add_lease_non_existent(self): + """ + If the storage index doesn't exist, adding the lease silently does nothing. + """ + storage_index = new_storage_index() + self.assertEqual(list(self.server.get_leases(storage_index)), []) + + renew_secret = new_secret() + cancel_secret = new_secret() + + # Add a lease: + yield self.storage_client.add_lease(storage_index, renew_secret, cancel_secret) + self.assertEqual(list(self.server.get_leases(storage_index)), []) + @inlineCallbacks def test_add_lease_renewal(self): """ @@ -854,6 +870,23 @@ class IStorageServerMutableAPIsTestsMixin(object): {0: [b"abcdefg"], 1: [b"0123456"], 2: [b"9876543"]}, ) + @inlineCallbacks + def test_slot_readv_unknown_storage_index(self): + """ + With unknown storage index, ``IStorageServer.slot_readv()`` returns + empty dict. + """ + storage_index = new_storage_index() + reads = yield self.storage_client.slot_readv( + storage_index, + shares=[], + readv=[(0, 7)], + ) + self.assertEqual( + reads, + {}, + ) + @inlineCallbacks def create_slot(self): """Create a slot with sharenum 0.""" @@ -883,6 +916,19 @@ class IStorageServerMutableAPIsTestsMixin(object): b"mutable", storage_index, 0, b"ono" ) + @inlineCallbacks + def test_advise_corrupt_share_unknown_share_number(self): + """ + Calling ``advise_corrupt_share()`` on a mutable share with an unknown + share number does not result in error (other behavior is opaque at this + level of abstraction). + """ + secrets, storage_index = yield self.create_slot() + + yield self.storage_client.advise_corrupt_share( + b"mutable", storage_index, 999, b"ono" + ) + @inlineCallbacks def test_STARAW_create_lease(self): """ @@ -998,7 +1044,10 @@ class _SharedMixin(SystemTestMixin): SKIP_TESTS = set() # type: Set[str] def _get_istorage_server(self): - raise NotImplementedError("implement in subclass") + native_server = next(iter(self.clients[0].storage_broker.get_known_servers())) + client = native_server.get_storage_server() + self.assertTrue(IStorageServer.providedBy(client)) + return client @inlineCallbacks def setUp(self): @@ -1021,7 +1070,7 @@ class _SharedMixin(SystemTestMixin): self._clock = Clock() self._clock.advance(123456) self.server._clock = self._clock - self.storage_client = yield self._get_istorage_server() + self.storage_client = self._get_istorage_server() def fake_time(self): """Return the current fake, test-controlled, time.""" @@ -1037,74 +1086,29 @@ class _SharedMixin(SystemTestMixin): yield SystemTestMixin.tearDown(self) -class _FoolscapMixin(_SharedMixin): - """Run tests on Foolscap version of ``IStorageServer``.""" - - def _get_native_server(self): - return next(iter(self.clients[0].storage_broker.get_known_servers())) - - def _get_istorage_server(self): - client = self._get_native_server().get_storage_server() - self.assertTrue(IStorageServer.providedBy(client)) - return succeed(client) - - -class _HTTPMixin(_SharedMixin): - """Run tests on the HTTP version of ``IStorageServer``.""" - - def setUp(self): - self._port_assigner = SameProcessStreamEndpointAssigner() - self._port_assigner.setUp() - self.addCleanup(self._port_assigner.tearDown) - return _SharedMixin.setUp(self) - - @inlineCallbacks - def _get_istorage_server(self): - swissnum = b"1234" - http_storage_server = HTTPServer(self.server, swissnum) - - # Listen on randomly assigned port, using self-signed cert: - private_key = generate_private_key() - certificate = generate_certificate(private_key) - _, endpoint_string = self._port_assigner.assign(reactor) - nurl, listening_port = yield listen_tls( - http_storage_server, - "127.0.0.1", - serverFromString(reactor, endpoint_string), - private_key_to_file(FilePath(self.mktemp()), private_key), - cert_to_file(FilePath(self.mktemp()), certificate), - ) - self.addCleanup(listening_port.stopListening) - - # Create HTTP client with non-persistent connections, so we don't leak - # state across tests: - returnValue( - _HTTPStorageServer.from_http_client( - StorageClient.from_nurl(nurl, reactor, persistent=False) - ) - ) - - # Eventually should also: - # self.assertTrue(IStorageServer.providedBy(client)) - - class FoolscapSharedAPIsTests( - _FoolscapMixin, IStorageServerSharedAPIsTestsMixin, AsyncTestCase + _SharedMixin, IStorageServerSharedAPIsTestsMixin, AsyncTestCase ): """Foolscap-specific tests for shared ``IStorageServer`` APIs.""" + FORCE_FOOLSCAP_FOR_STORAGE = True + class HTTPSharedAPIsTests( - _HTTPMixin, IStorageServerSharedAPIsTestsMixin, AsyncTestCase + _SharedMixin, IStorageServerSharedAPIsTestsMixin, AsyncTestCase ): """HTTP-specific tests for shared ``IStorageServer`` APIs.""" + FORCE_FOOLSCAP_FOR_STORAGE = False + class FoolscapImmutableAPIsTests( - _FoolscapMixin, IStorageServerImmutableAPIsTestsMixin, AsyncTestCase + _SharedMixin, IStorageServerImmutableAPIsTestsMixin, AsyncTestCase ): """Foolscap-specific tests for immutable ``IStorageServer`` APIs.""" + FORCE_FOOLSCAP_FOR_STORAGE = True + def test_disconnection(self): """ If we disconnect in the middle of writing to a bucket, all data is @@ -1127,32 +1131,29 @@ class FoolscapImmutableAPIsTests( """ current = self.storage_client yield self.bounce_client(0) - self.storage_client = self._get_native_server().get_storage_server() + self.storage_client = self._get_istorage_server() assert self.storage_client is not current class HTTPImmutableAPIsTests( - _HTTPMixin, IStorageServerImmutableAPIsTestsMixin, AsyncTestCase + _SharedMixin, IStorageServerImmutableAPIsTestsMixin, AsyncTestCase ): """HTTP-specific tests for immutable ``IStorageServer`` APIs.""" + FORCE_FOOLSCAP_FOR_STORAGE = False + class FoolscapMutableAPIsTests( - _FoolscapMixin, IStorageServerMutableAPIsTestsMixin, AsyncTestCase + _SharedMixin, IStorageServerMutableAPIsTestsMixin, AsyncTestCase ): """Foolscap-specific tests for mutable ``IStorageServer`` APIs.""" + FORCE_FOOLSCAP_FOR_STORAGE = True + class HTTPMutableAPIsTests( - _HTTPMixin, IStorageServerMutableAPIsTestsMixin, AsyncTestCase + _SharedMixin, IStorageServerMutableAPIsTestsMixin, AsyncTestCase ): """HTTP-specific tests for mutable ``IStorageServer`` APIs.""" - # TODO will be implemented in later tickets - SKIP_TESTS = { - "test_STARAW_write_enabler_must_match", - "test_add_lease_renewal", - "test_add_new_lease", - "test_advise_corrupt_share", - "test_slot_readv_no_shares", - } + FORCE_FOOLSCAP_FOR_STORAGE = False diff --git a/src/allmydata/test/test_protocol_switch.py b/src/allmydata/test/test_protocol_switch.py new file mode 100644 index 000000000..4906896dc --- /dev/null +++ b/src/allmydata/test/test_protocol_switch.py @@ -0,0 +1,43 @@ +""" +Unit tests for ``allmydata.protocol_switch``. + +By its nature, most of the testing needs to be end-to-end; essentially any test +that uses real Foolscap (``test_system.py``, integration tests) ensures +Foolscap still works. ``test_istorageserver.py`` tests the HTTP support. +""" + +from foolscap.negotiate import Negotiation + +from .common import TestCase +from ..protocol_switch import _PretendToBeNegotiation + + +class UtilityTests(TestCase): + """Tests for utilities in the protocol switch code.""" + + def test_metaclass(self): + """ + A class that has the ``_PretendToBeNegotiation`` metaclass will support + ``isinstance()``'s normal semantics on its own instances, but will also + indicate that ``Negotiation`` instances are its instances. + """ + + class Parent(metaclass=_PretendToBeNegotiation): + pass + + class Child(Parent): + pass + + class Other: + pass + + p = Parent() + self.assertIsInstance(p, Parent) + self.assertIsInstance(Negotiation(), Parent) + self.assertNotIsInstance(Other(), Parent) + + c = Child() + self.assertIsInstance(c, Child) + self.assertIsInstance(c, Parent) + self.assertIsInstance(Negotiation(), Child) + self.assertNotIsInstance(Other(), Child) diff --git a/src/allmydata/test/test_repairer.py b/src/allmydata/test/test_repairer.py index 88696000c..8545b1cf4 100644 --- a/src/allmydata/test/test_repairer.py +++ b/src/allmydata/test/test_repairer.py @@ -251,6 +251,12 @@ class Verifier(GridTestMixin, unittest.TestCase, RepairTestMixin): self.judge_invisible_corruption) def test_corrupt_ueb(self): + # Note that in some rare situations this might fail, specifically if + # the length of the UEB is corrupted to be a value that is bigger than + # the size but less than 2000, it might not get caught... But that's + # mostly because in that case it doesn't meaningfully corrupt it. See + # _get_uri_extension_the_old_way() in layout.py for where the 2000 + # number comes from. self.basedir = "repairer/Verifier/corrupt_ueb" return self._help_test_verify(common._corrupt_uri_extension, self.judge_invisible_corruption) @@ -717,7 +723,7 @@ class Repairer(GridTestMixin, unittest.TestCase, RepairTestMixin, ss = self.g.servers_by_number[0] # we want to delete the share corresponding to the server # we're making not-respond - share = next(ss._get_bucket_shares(self.c0_filenode.get_storage_index()))[0] + share = next(ss.get_shares(self.c0_filenode.get_storage_index()))[0] self.delete_shares_numbered(self.uri, [share]) return self.c0_filenode.check_and_repair(Monitor()) d.addCallback(_then) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 3eb6b8a34..74e3f803e 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -42,16 +42,19 @@ from twisted.trial import unittest from twisted.internet import reactor from twisted.python import usage +from twisted.python.runtime import platform from twisted.internet.defer import ( inlineCallbacks, DeferredList, ) from twisted.python.filepath import FilePath -from twisted.python.runtime import ( - platform, -) from allmydata.util import fileutil, pollmixin from allmydata.util.encodingutil import unicode_to_argv +from allmydata.util.pid import ( + check_pid_process, + _pidfile_to_lockpath, + ProcessInTheWay, +) from allmydata.test import common_util import allmydata from allmydata.scripts.runner import ( @@ -418,9 +421,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): tahoe.active() - # We don't keep track of PIDs in files on Windows. - if not platform.isWindows(): - self.assertTrue(tahoe.twistd_pid_file.exists()) + self.assertTrue(tahoe.twistd_pid_file.exists()) self.assertTrue(tahoe.node_url_file.exists()) # rm this so we can detect when the second incarnation is ready @@ -493,9 +494,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): # change on restart storage_furl = fileutil.read(tahoe.storage_furl_file.path) - # We don't keep track of PIDs in files on Windows. - if not platform.isWindows(): - self.assertTrue(tahoe.twistd_pid_file.exists()) + self.assertTrue(tahoe.twistd_pid_file.exists()) # rm this so we can detect when the second incarnation is ready tahoe.node_url_file.remove() @@ -513,22 +512,23 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): fileutil.read(tahoe.storage_furl_file.path), ) - if not platform.isWindows(): - self.assertTrue( - tahoe.twistd_pid_file.exists(), - "PID file ({}) didn't exist when we expected it to. " - "These exist: {}".format( - tahoe.twistd_pid_file, - tahoe.twistd_pid_file.parent().listdir(), - ), - ) + self.assertTrue( + tahoe.twistd_pid_file.exists(), + "PID file ({}) didn't exist when we expected it to. " + "These exist: {}".format( + tahoe.twistd_pid_file, + tahoe.twistd_pid_file.parent().listdir(), + ), + ) yield tahoe.stop_and_wait() + # twistd.pid should be gone by now -- except on Windows, where + # killing a subprocess immediately exits with no chance for + # any shutdown code (that is, no Twisted shutdown hooks can + # run). if not platform.isWindows(): - # twistd.pid should be gone by now. self.assertFalse(tahoe.twistd_pid_file.exists()) - def _remove(self, res, file): fileutil.remove(file) return res @@ -610,8 +610,9 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): ), ) + # It should not be running (but windows shutdown can't run + # code so the PID file still exists there). if not platform.isWindows(): - # It should not be running. self.assertFalse(tahoe.twistd_pid_file.exists()) # Wait for the operation to *complete*. If we got this far it's @@ -621,3 +622,42 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): # What's left is a perfect indicator that the process has exited and # we won't get blamed for leaving the reactor dirty. yield client_running + + +class PidFileLocking(SyncTestCase): + """ + Direct tests for allmydata.util.pid functions + """ + + def test_locking(self): + """ + Fail to create a pidfile if another process has the lock already. + """ + # this can't just be "our" process because the locking library + # allows the same process to acquire a lock multiple times. + pidfile = FilePath(self.mktemp()) + lockfile = _pidfile_to_lockpath(pidfile) + + with open("other_lock.py", "w") as f: + f.write( + "\n".join([ + "import filelock, time, sys", + "with filelock.FileLock(sys.argv[1], timeout=1):", + " sys.stdout.write('.\\n')", + " sys.stdout.flush()", + " time.sleep(10)", + ]) + ) + proc = Popen( + [sys.executable, "other_lock.py", lockfile.path], + stdout=PIPE, + stderr=PIPE, + ) + # make sure our subprocess has had time to acquire the lock + # for sure (from the "." it prints) + proc.stdout.read(2) + + # acquiring the same lock should fail; it is locked by the subprocess + with self.assertRaises(ProcessInTheWay): + check_pid_process(pidfile) + proc.terminate() diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index b37f74c24..134609f81 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -463,7 +463,7 @@ class BucketProxy(unittest.TestCase): block_size=10, num_segments=5, num_share_hashes=3, - uri_extension_size_max=500) + uri_extension_size=500) self.failUnless(interfaces.IStorageBucketWriter.providedBy(bp), bp) def _do_test_readwrite(self, name, header_size, wbp_class, rbp_class): @@ -494,7 +494,7 @@ class BucketProxy(unittest.TestCase): block_size=25, num_segments=4, num_share_hashes=3, - uri_extension_size_max=len(uri_extension)) + uri_extension_size=len(uri_extension)) d = bp.put_header() d.addCallback(lambda res: bp.put_block(0, b"a"*25)) @@ -688,6 +688,19 @@ class Server(unittest.TestCase): writer.abort() self.failUnlessEqual(ss.allocated_size(), 0) + def test_immutable_length(self): + """ + ``get_immutable_share_length()`` returns the length of an immutable + share, as does ``BucketWriter.get_length()``.. + """ + ss = self.create("test_immutable_length") + _, writers = self.allocate(ss, b"allocate", [22], 75) + bucket = writers[22] + bucket.write(0, b"X" * 75) + bucket.close() + self.assertEqual(ss.get_immutable_share_length(b"allocate", 22), 75) + self.assertEqual(ss.get_buckets(b"allocate")[22].get_length(), 75) + def test_allocate(self): ss = self.create("test_allocate") @@ -766,7 +779,7 @@ class Server(unittest.TestCase): writer.close() # It should have a lease granted at the current time. - shares = dict(ss._get_bucket_shares(storage_index)) + shares = dict(ss.get_shares(storage_index)) self.assertEqual( [first_lease], list( @@ -789,7 +802,7 @@ class Server(unittest.TestCase): writer.close() # The first share's lease expiration time is unchanged. - shares = dict(ss._get_bucket_shares(storage_index)) + shares = dict(ss.get_shares(storage_index)) self.assertEqual( [first_lease], list( @@ -1315,6 +1328,64 @@ class MutableServer(unittest.TestCase): self.failUnless(isinstance(readv_data, dict)) self.failUnlessEqual(len(readv_data), 0) + def test_enumerate_mutable_shares(self): + """ + ``StorageServer.enumerate_mutable_shares()`` returns a set of share + numbers for the given storage index, or an empty set if it does not + exist at all. + """ + ss = self.create("test_enumerate_mutable_shares") + + # Initially, nothing exists: + empty = ss.enumerate_mutable_shares(b"si1") + + self.allocate(ss, b"si1", b"we1", b"le1", [0, 1, 4, 2], 12) + shares0_1_2_4 = ss.enumerate_mutable_shares(b"si1") + + # Remove share 2, by setting size to 0: + secrets = (self.write_enabler(b"we1"), + self.renew_secret(b"le1"), + self.cancel_secret(b"le1")) + ss.slot_testv_and_readv_and_writev(b"si1", secrets, {2: ([], [], 0)}, []) + shares0_1_4 = ss.enumerate_mutable_shares(b"si1") + self.assertEqual( + (empty, shares0_1_2_4, shares0_1_4), + (set(), {0, 1, 2, 4}, {0, 1, 4}) + ) + + def test_mutable_share_length(self): + """``get_mutable_share_length()`` returns the length of the share.""" + ss = self.create("test_mutable_share_length") + self.allocate(ss, b"si1", b"we1", b"le1", [16], 23) + ss.slot_testv_and_readv_and_writev( + b"si1", (self.write_enabler(b"we1"), + self.renew_secret(b"le1"), + self.cancel_secret(b"le1")), + {16: ([], [(0, b"x" * 23)], None)}, + [] + ) + self.assertEqual(ss.get_mutable_share_length(b"si1", 16), 23) + + def test_mutable_share_length_unknown(self): + """ + ``get_mutable_share_length()`` raises a ``KeyError`` on unknown shares. + """ + ss = self.create("test_mutable_share_length_unknown") + self.allocate(ss, b"si1", b"we1", b"le1", [16], 23) + ss.slot_testv_and_readv_and_writev( + b"si1", (self.write_enabler(b"we1"), + self.renew_secret(b"le1"), + self.cancel_secret(b"le1")), + {16: ([], [(0, b"x" * 23)], None)}, + [] + ) + with self.assertRaises(KeyError): + # Wrong share number. + ss.get_mutable_share_length(b"si1", 17) + with self.assertRaises(KeyError): + # Wrong storage index + ss.get_mutable_share_length(b"unknown", 16) + def test_bad_magic(self): ss = self.create("test_bad_magic") self.allocate(ss, b"si1", b"we1", next(self._lease_secret), set([0]), 10) diff --git a/src/allmydata/test/test_storage_http.py b/src/allmydata/test/test_storage_http.py index df781012e..8dbe18545 100644 --- a/src/allmydata/test/test_storage_http.py +++ b/src/allmydata/test/test_storage_http.py @@ -1,20 +1,38 @@ """ Tests for HTTP storage client + server. + +The tests here are synchronous and don't involve running a real reactor. This +works, but has some caveats when it comes to testing HTTP endpoints: + +* Some HTTP endpoints are synchronous, some are not. +* For synchronous endpoints, the result is immediately available on the + ``Deferred`` coming out of ``StubTreq``. +* For asynchronous endpoints, you need to use ``StubTreq.flush()`` and + iterate the fake in-memory clock/reactor to advance time . + +So for HTTP endpoints, you should use ``HttpTestFixture.result_of_with_flush()`` +which handles both, and patches and moves forward the global Twisted +``Cooperator`` since that is used to drive pull producers. This is, +sadly, an internal implementation detail of Twisted being leaked to tests... + +For definitely synchronous calls, you can just use ``result_of()``. """ from base64 import b64encode from contextlib import contextmanager from os import urandom - +from typing import Union, Callable, Tuple, Iterable from cbor2 import dumps from pycddl import ValidationError as CDDLValidationError from hypothesis import assume, given, strategies as st -from fixtures import Fixture, TempDir +from fixtures import Fixture, TempDir, MonkeyPatch from treq.testing import StubTreq from klein import Klein from hyperlink import DecodedURL from collections_extended import RangeMap -from twisted.internet.task import Clock +from twisted.internet.task import Clock, Cooperator +from twisted.internet.interfaces import IReactorTime +from twisted.internet.defer import CancelledError, Deferred from twisted.web import http from twisted.web.http_headers import Headers from werkzeug import routing @@ -23,6 +41,7 @@ from werkzeug.exceptions import NotFound as WNotFound from .common import SyncTestCase from ..storage.http_common import get_content_type, CBOR_MIME_TYPE from ..storage.common import si_b2a +from ..storage.lease import LeaseInfo from ..storage.server import StorageServer from ..storage.http_server import ( HTTPServer, @@ -40,6 +59,13 @@ from ..storage.http_client import ( UploadProgress, StorageClientGeneral, _encode_si, + StorageClientMutables, + TestWriteVectors, + WriteVector, + ReadVector, + ReadTestWriteResult, + TestVector, + limited_content, ) @@ -211,9 +237,17 @@ class RouteConverterTests(SyncTestCase): SWISSNUM_FOR_TEST = b"abcd" +def gen_bytes(length: int) -> bytes: + """Generate bytes to the given length.""" + result = (b"0123456789abcdef" * ((length // 16) + 1))[:length] + assert len(result) == length + return result + + class TestApp(object): """HTTP API for testing purposes.""" + clock: IReactorTime _app = Klein() _swissnum = SWISSNUM_FOR_TEST # Match what the test client is using @@ -224,12 +258,36 @@ class TestApp(object): else: return "BAD: {}".format(authorization) - @_authorized_route(_app, set(), "/v1/version", methods=["GET"]) + @_authorized_route(_app, set(), "/storage/v1/version", methods=["GET"]) def bad_version(self, request, authorization): """Return version result that violates the expected schema.""" request.setHeader("content-type", CBOR_MIME_TYPE) return dumps({"garbage": 123}) + @_authorized_route(_app, set(), "/bytes/", methods=["GET"]) + def generate_bytes(self, request, authorization, length): + """Return bytes to the given length using ``gen_bytes()``.""" + return gen_bytes(length) + + @_authorized_route(_app, set(), "/slowly_never_finish_result", methods=["GET"]) + def slowly_never_finish_result(self, request, authorization): + """ + Send data immediately, after 59 seconds, after another 59 seconds, and then + never again, without finishing the response. + """ + request.write(b"a") + self.clock.callLater(59, request.write, b"b") + self.clock.callLater(59 + 59, request.write, b"c") + return Deferred() + + @_authorized_route(_app, set(), "/die_unfinished", methods=["GET"]) + def die(self, request, authorization): + """ + Dies half-way. + """ + request.transport.loseConnection() + return Deferred() + def result_of(d): """ @@ -255,14 +313,25 @@ class CustomHTTPServerTests(SyncTestCase): def setUp(self): super(CustomHTTPServerTests, self).setUp() + StorageClient.start_test_mode( + lambda pool: self.addCleanup(pool.closeCachedConnections) + ) + self.addCleanup(StorageClient.stop_test_mode) # Could be a fixture, but will only be used in this test class so not # going to bother: self._http_server = TestApp() + treq = StubTreq(self._http_server._app.resource()) self.client = StorageClient( DecodedURL.from_text("http://127.0.0.1"), SWISSNUM_FOR_TEST, - treq=StubTreq(self._http_server._app.resource()), + treq=treq, + # We're using a Treq private API to get the reactor, alas, but only + # in a test, so not going to worry about it too much. This would be + # fixed if https://github.com/twisted/treq/issues/226 were ever + # fixed. + clock=treq._agent._memoryReactor, ) + self._http_server.clock = self.client._clock def test_authorization_enforcement(self): """ @@ -295,6 +364,89 @@ class CustomHTTPServerTests(SyncTestCase): with self.assertRaises(CDDLValidationError): result_of(client.get_version()) + @given(length=st.integers(min_value=1, max_value=1_000_000)) + def test_limited_content_fits(self, length): + """ + ``http_client.limited_content()`` returns the body if it is less than + the max length. + """ + for at_least_length in (length, length + 1, length + 1000, length + 100_000): + response = result_of( + self.client.request( + "GET", + f"http://127.0.0.1/bytes/{length}", + ) + ) + + self.assertEqual( + result_of( + limited_content(response, self._http_server.clock, at_least_length) + ).read(), + gen_bytes(length), + ) + + @given(length=st.integers(min_value=10, max_value=1_000_000)) + def test_limited_content_does_not_fit(self, length): + """ + If the body is longer than than max length, + ``http_client.limited_content()`` fails with a ``ValueError``. + """ + for too_short in (length - 1, 5): + response = result_of( + self.client.request( + "GET", + f"http://127.0.0.1/bytes/{length}", + ) + ) + + with self.assertRaises(ValueError): + result_of(limited_content(response, self._http_server.clock, too_short)) + + def test_limited_content_silence_causes_timeout(self): + """ + ``http_client.limited_content() times out if it receives no data for 60 + seconds. + """ + response = result_of( + self.client.request( + "GET", + "http://127.0.0.1/slowly_never_finish_result", + ) + ) + + body_deferred = limited_content(response, self._http_server.clock, 4) + result = [] + error = [] + body_deferred.addCallbacks(result.append, error.append) + + for i in range(59 + 59 + 60): + self.assertEqual((result, error), ([], [])) + self._http_server.clock.advance(1) + # Push data between in-memory client and in-memory server: + self.client._treq._agent.flush() + + # After 59 (second write) + 59 (third write) + 60 seconds (quiescent + # timeout) the limited_content() response times out. + self.assertTrue(error) + with self.assertRaises(CancelledError): + error[0].raiseException() + + def test_limited_content_cancels_timeout_on_failed_response(self): + """ + If the response fails somehow, the timeout is still cancelled. + """ + response = result_of( + self.client.request( + "GET", + "http://127.0.0.1/die", + ) + ) + + d = limited_content(response, self._http_server.clock, 4) + with self.assertRaises(ValueError): + result_of(d) + self.assertEqual(len(self._http_server.clock.getDelayedCalls()), 0) + class HttpTestFixture(Fixture): """ @@ -303,16 +455,61 @@ class HttpTestFixture(Fixture): """ def _setUp(self): + StorageClient.start_test_mode( + lambda pool: self.addCleanup(pool.closeCachedConnections) + ) + self.addCleanup(StorageClient.stop_test_mode) self.clock = Clock() self.tempdir = self.useFixture(TempDir()) + # The global Cooperator used by Twisted (a) used by pull producers in + # twisted.web, (b) is driven by a real reactor. We want to push time + # forward ourselves since we rely on pull producers in the HTTP storage + # server. + self.mock = self.useFixture( + MonkeyPatch( + "twisted.internet.task._theCooperator", + Cooperator(scheduler=lambda c: self.clock.callLater(0.000001, c)), + ) + ) self.storage_server = StorageServer( self.tempdir.path, b"\x00" * 20, clock=self.clock ) self.http_server = HTTPServer(self.storage_server, SWISSNUM_FOR_TEST) + self.treq = StubTreq(self.http_server.get_resource()) self.client = StorageClient( DecodedURL.from_text("http://127.0.0.1"), SWISSNUM_FOR_TEST, - treq=StubTreq(self.http_server.get_resource()), + treq=self.treq, + clock=self.clock, + ) + + def result_of_with_flush(self, d): + """ + Like ``result_of``, but supports fake reactor and ``treq`` testing + infrastructure necessary to support asynchronous HTTP server endpoints. + """ + result = [] + error = [] + d.addCallbacks(result.append, error.append) + + # Check for synchronous HTTP endpoint handler: + if result: + return result[0] + if error: + error[0].raiseException() + + # OK, no result yet, probably async HTTP endpoint handler, so advance + # time, flush treq, and try again: + for i in range(100): + self.clock.advance(0.001) + self.treq.flush() + if result: + return result[0] + if error: + error[0].raiseException() + raise RuntimeError( + "We expected given Deferred to have result already, but it wasn't. " + + "This is probably a test design issue." ) @@ -368,10 +565,11 @@ class GenericHTTPAPITests(SyncTestCase): DecodedURL.from_text("http://127.0.0.1"), b"something wrong", treq=StubTreq(self.http.http_server.get_resource()), + clock=self.http.clock, ) ) with assert_fails_with_http_code(self, http.UNAUTHORIZED): - result_of(client.get_version()) + self.http.result_of_with_flush(client.get_version()) def test_unsupported_mime_type(self): """ @@ -382,7 +580,7 @@ class GenericHTTPAPITests(SyncTestCase): StorageClientWithHeadersOverride(self.http.client, {"accept": "image/gif"}) ) with assert_fails_with_http_code(self, http.NOT_ACCEPTABLE): - result_of(client.get_version()) + self.http.result_of_with_flush(client.get_version()) def test_version(self): """ @@ -392,7 +590,7 @@ class GenericHTTPAPITests(SyncTestCase): might change across calls. """ client = StorageClientGeneral(self.http.client) - version = result_of(client.get_version()) + version = self.http.result_of_with_flush(client.get_version()) version[b"http://allmydata.org/tahoe/protocols/storage/v1"].pop( b"available-space" ) @@ -422,11 +620,11 @@ class GenericHTTPAPITests(SyncTestCase): lease_secret = urandom(32) storage_index = urandom(16) url = self.http.client.relative_url( - "/v1/immutable/" + _encode_si(storage_index) + "/storage/v1/immutable/" + _encode_si(storage_index) ) message = {"bad-message": "missing expected keys"} - response = result_of( + response = self.http.result_of_with_flush( self.http.client.request( "POST", url, @@ -448,6 +646,7 @@ class ImmutableHTTPAPITests(SyncTestCase): super(ImmutableHTTPAPITests, self).setUp() self.http = self.useFixture(HttpTestFixture()) self.imm_client = StorageClientImmutables(self.http.client) + self.general_client = StorageClientGeneral(self.http.client) def create_upload(self, share_numbers, length): """ @@ -458,7 +657,7 @@ class ImmutableHTTPAPITests(SyncTestCase): upload_secret = urandom(32) lease_secret = urandom(32) storage_index = urandom(16) - created = result_of( + created = self.http.result_of_with_flush( self.imm_client.create( storage_index, share_numbers, @@ -502,42 +701,42 @@ class ImmutableHTTPAPITests(SyncTestCase): expected_data[offset : offset + length], ) - upload_progress = result_of(write(10, 10)) + upload_progress = self.http.result_of_with_flush(write(10, 10)) self.assertEqual( upload_progress, UploadProgress(finished=False, required=remaining) ) - upload_progress = result_of(write(30, 10)) + upload_progress = self.http.result_of_with_flush(write(30, 10)) self.assertEqual( upload_progress, UploadProgress(finished=False, required=remaining) ) - upload_progress = result_of(write(50, 10)) + upload_progress = self.http.result_of_with_flush(write(50, 10)) self.assertEqual( upload_progress, UploadProgress(finished=False, required=remaining) ) # Then, an overlapping write with matching data (15-35): - upload_progress = result_of(write(15, 20)) + upload_progress = self.http.result_of_with_flush(write(15, 20)) self.assertEqual( upload_progress, UploadProgress(finished=False, required=remaining) ) # Now fill in the holes: - upload_progress = result_of(write(0, 10)) + upload_progress = self.http.result_of_with_flush(write(0, 10)) self.assertEqual( upload_progress, UploadProgress(finished=False, required=remaining) ) - upload_progress = result_of(write(40, 10)) + upload_progress = self.http.result_of_with_flush(write(40, 10)) self.assertEqual( upload_progress, UploadProgress(finished=False, required=remaining) ) - upload_progress = result_of(write(60, 40)) + upload_progress = self.http.result_of_with_flush(write(60, 40)) self.assertEqual( upload_progress, UploadProgress(finished=True, required=RangeMap()) ) # We can now read: for offset, length in [(0, 100), (10, 19), (99, 1), (49, 200)]: - downloaded = result_of( + downloaded = self.http.result_of_with_flush( self.imm_client.read_share_chunk(storage_index, 1, offset, length) ) self.assertEqual(downloaded, expected_data[offset : offset + length]) @@ -549,7 +748,7 @@ class ImmutableHTTPAPITests(SyncTestCase): """ (upload_secret, _, storage_index, _) = self.create_upload({1}, 100) with assert_fails_with_http_code(self, http.UNAUTHORIZED): - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 1, @@ -571,7 +770,7 @@ class ImmutableHTTPAPITests(SyncTestCase): ) # Write half of share 1 - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 1, @@ -585,7 +784,7 @@ class ImmutableHTTPAPITests(SyncTestCase): # existing shares, this call shouldn't overwrite the existing # work-in-progress. upload_secret2 = b"x" * 2 - created2 = result_of( + created2 = self.http.result_of_with_flush( self.imm_client.create( storage_index, {1, 4, 6}, @@ -599,7 +798,7 @@ class ImmutableHTTPAPITests(SyncTestCase): # Write second half of share 1 self.assertTrue( - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 1, @@ -612,14 +811,14 @@ class ImmutableHTTPAPITests(SyncTestCase): # The upload of share 1 succeeded, demonstrating that second create() # call didn't overwrite work-in-progress. - downloaded = result_of( + downloaded = self.http.result_of_with_flush( self.imm_client.read_share_chunk(storage_index, 1, 0, 100) ) self.assertEqual(downloaded, b"a" * 50 + b"b" * 50) # We can successfully upload the shares created with the second upload secret. self.assertTrue( - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 4, @@ -637,11 +836,14 @@ class ImmutableHTTPAPITests(SyncTestCase): (upload_secret, _, storage_index, created) = self.create_upload({1, 2, 3}, 10) # Initially there are no shares: - self.assertEqual(result_of(self.imm_client.list_shares(storage_index)), set()) + self.assertEqual( + self.http.result_of_with_flush(self.imm_client.list_shares(storage_index)), + set(), + ) # Upload shares 1 and 3: for share_number in [1, 3]: - progress = result_of( + progress = self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, share_number, @@ -653,7 +855,10 @@ class ImmutableHTTPAPITests(SyncTestCase): self.assertTrue(progress.finished) # Now shares 1 and 3 exist: - self.assertEqual(result_of(self.imm_client.list_shares(storage_index)), {1, 3}) + self.assertEqual( + self.http.result_of_with_flush(self.imm_client.list_shares(storage_index)), + {1, 3}, + ) def test_upload_bad_content_range(self): """ @@ -671,7 +876,7 @@ class ImmutableHTTPAPITests(SyncTestCase): with assert_fails_with_http_code( self, http.REQUESTED_RANGE_NOT_SATISFIABLE ): - result_of( + self.http.result_of_with_flush( client.write_share_chunk( storage_index, 1, @@ -691,7 +896,10 @@ class ImmutableHTTPAPITests(SyncTestCase): Listing unknown storage index's shares results in empty list of shares. """ storage_index = bytes(range(16)) - self.assertEqual(result_of(self.imm_client.list_shares(storage_index)), set()) + self.assertEqual( + self.http.result_of_with_flush(self.imm_client.list_shares(storage_index)), + set(), + ) def test_upload_non_existent_storage_index(self): """ @@ -702,7 +910,7 @@ class ImmutableHTTPAPITests(SyncTestCase): def unknown_check(storage_index, share_number): with assert_fails_with_http_code(self, http.NOT_FOUND): - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, share_number, @@ -723,7 +931,7 @@ class ImmutableHTTPAPITests(SyncTestCase): stored separately and can be downloaded separately. """ (upload_secret, _, storage_index, _) = self.create_upload({1, 2}, 10) - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 1, @@ -732,7 +940,7 @@ class ImmutableHTTPAPITests(SyncTestCase): b"1" * 10, ) ) - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 2, @@ -742,11 +950,15 @@ class ImmutableHTTPAPITests(SyncTestCase): ) ) self.assertEqual( - result_of(self.imm_client.read_share_chunk(storage_index, 1, 0, 10)), + self.http.result_of_with_flush( + self.imm_client.read_share_chunk(storage_index, 1, 0, 10) + ), b"1" * 10, ) self.assertEqual( - result_of(self.imm_client.read_share_chunk(storage_index, 2, 0, 10)), + self.http.result_of_with_flush( + self.imm_client.read_share_chunk(storage_index, 2, 0, 10) + ), b"2" * 10, ) @@ -758,7 +970,7 @@ class ImmutableHTTPAPITests(SyncTestCase): (upload_secret, _, storage_index, created) = self.create_upload({1}, 100) # Write: - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 1, @@ -770,7 +982,7 @@ class ImmutableHTTPAPITests(SyncTestCase): # Conflicting write: with assert_fails_with_http_code(self, http.CONFLICT): - result_of( + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, 1, @@ -780,34 +992,445 @@ class ImmutableHTTPAPITests(SyncTestCase): ) ) - def upload(self, share_number, data_length=26): + def test_timed_out_upload_allows_reupload(self): """ - Create a share, return (storage_index, uploaded_data). + If an in-progress upload times out, it is cancelled altogether, + allowing a new upload to occur. """ - uploaded_data = (b"abcdefghijklmnopqrstuvwxyz" * ((data_length // 26) + 1))[ - :data_length - ] - (upload_secret, _, storage_index, _) = self.create_upload( - {share_number}, data_length + self._test_abort_or_timed_out_upload_to_existing_storage_index( + lambda **kwargs: self.http.clock.advance(30 * 60 + 1) ) - result_of( + + def test_abort_upload_allows_reupload(self): + """ + If an in-progress upload is aborted, it is cancelled altogether, + allowing a new upload to occur. + """ + + def abort(storage_index, share_number, upload_secret): + return self.http.result_of_with_flush( + self.imm_client.abort_upload(storage_index, share_number, upload_secret) + ) + + self._test_abort_or_timed_out_upload_to_existing_storage_index(abort) + + def _test_abort_or_timed_out_upload_to_existing_storage_index(self, cancel_upload): + """Start uploading to an existing storage index that then times out or aborts. + + Re-uploading should work. + """ + # Start an upload: + (upload_secret, _, storage_index, _) = self.create_upload({1}, 100) + self.http.result_of_with_flush( self.imm_client.write_share_chunk( storage_index, - share_number, + 1, + upload_secret, + 0, + b"123", + ) + ) + + # Now, the upload is cancelled somehow: + cancel_upload( + storage_index=storage_index, upload_secret=upload_secret, share_number=1 + ) + + # Now we can create a new share with the same storage index without + # complaint: + upload_secret = urandom(32) + lease_secret = urandom(32) + created = self.http.result_of_with_flush( + self.imm_client.create( + storage_index, + {1}, + 100, + upload_secret, + lease_secret, + lease_secret, + ) + ) + self.assertEqual(created.allocated, {1}) + + # And write to it, too: + self.http.result_of_with_flush( + self.imm_client.write_share_chunk( + storage_index, + 1, + upload_secret, + 0, + b"ABC", + ) + ) + + def test_unknown_aborts(self): + """ + Aborting uploads with an unknown storage index or share number will + result 404 HTTP response code. + """ + (upload_secret, _, storage_index, _) = self.create_upload({1}, 100) + + for si, num in [(storage_index, 3), (b"x" * 16, 1)]: + with assert_fails_with_http_code(self, http.NOT_FOUND): + self.http.result_of_with_flush( + self.imm_client.abort_upload(si, num, upload_secret) + ) + + def test_unauthorized_abort(self): + """ + An abort with the wrong key will return an unauthorized error, and will + not abort the upload. + """ + (upload_secret, _, storage_index, _) = self.create_upload({1}, 100) + + # Failed to abort becaues wrong upload secret: + with assert_fails_with_http_code(self, http.UNAUTHORIZED): + self.http.result_of_with_flush( + self.imm_client.abort_upload(storage_index, 1, upload_secret + b"X") + ) + + # We can still write to it: + self.http.result_of_with_flush( + self.imm_client.write_share_chunk( + storage_index, + 1, + upload_secret, + 0, + b"ABC", + ) + ) + + def test_too_late_abort(self): + """ + An abort of an already-fully-uploaded immutable will result in 405 + error and will not affect the immutable. + """ + uploaded_data = b"123" + (upload_secret, _, storage_index, _) = self.create_upload({0}, 3) + self.http.result_of_with_flush( + self.imm_client.write_share_chunk( + storage_index, + 0, upload_secret, 0, uploaded_data, ) ) - return storage_index, uploaded_data + + # Can't abort, we finished upload: + with assert_fails_with_http_code(self, http.NOT_ALLOWED): + self.http.result_of_with_flush( + self.imm_client.abort_upload(storage_index, 0, upload_secret) + ) + + # Abort didn't prevent reading: + self.assertEqual( + uploaded_data, + self.http.result_of_with_flush( + self.imm_client.read_share_chunk( + storage_index, + 0, + 0, + 3, + ) + ), + ) + + def test_lease_on_unknown_storage_index(self): + """ + An attempt to renew an unknown storage index will result in a HTTP 404. + """ + storage_index = urandom(16) + secret = b"A" * 32 + with assert_fails_with_http_code(self, http.NOT_FOUND): + self.http.result_of_with_flush( + self.general_client.add_or_renew_lease(storage_index, secret, secret) + ) + + +class MutableHTTPAPIsTests(SyncTestCase): + """Tests for mutable APIs.""" + + def setUp(self): + super(MutableHTTPAPIsTests, self).setUp() + self.http = self.useFixture(HttpTestFixture()) + self.mut_client = StorageClientMutables(self.http.client) + + def create_upload(self, data=b"abcdef"): + """ + Utility that creates shares 0 and 1 with bodies + ``{data}-{share_number}``. + """ + write_secret = urandom(32) + lease_secret = urandom(32) + storage_index = urandom(16) + self.http.result_of_with_flush( + self.mut_client.read_test_write_chunks( + storage_index, + write_secret, + lease_secret, + lease_secret, + { + 0: TestWriteVectors( + write_vectors=[WriteVector(offset=0, data=data + b"-0")] + ), + 1: TestWriteVectors( + write_vectors=[ + WriteVector(offset=0, data=data), + WriteVector(offset=len(data), data=b"-1"), + ] + ), + }, + [], + ) + ) + return storage_index, write_secret, lease_secret + + def test_write_can_be_read(self): + """ + Written data can be read using ``read_share_chunk``. + """ + storage_index, _, _ = self.create_upload() + data0 = self.http.result_of_with_flush( + self.mut_client.read_share_chunk(storage_index, 0, 1, 7) + ) + data1 = self.http.result_of_with_flush( + self.mut_client.read_share_chunk(storage_index, 1, 0, 8) + ) + self.assertEqual((data0, data1), (b"bcdef-0", b"abcdef-1")) + + def test_read_before_write(self): + """In combo read/test/write operation, reads happen before writes.""" + storage_index, write_secret, lease_secret = self.create_upload() + result = self.http.result_of_with_flush( + self.mut_client.read_test_write_chunks( + storage_index, + write_secret, + lease_secret, + lease_secret, + { + 0: TestWriteVectors( + write_vectors=[WriteVector(offset=1, data=b"XYZ")] + ), + }, + [ReadVector(0, 8)], + ) + ) + # Reads are from before the write: + self.assertEqual( + result, + ReadTestWriteResult( + success=True, reads={0: [b"abcdef-0"], 1: [b"abcdef-1"]} + ), + ) + # But the write did happen: + data0 = self.http.result_of_with_flush( + self.mut_client.read_share_chunk(storage_index, 0, 0, 8) + ) + data1 = self.http.result_of_with_flush( + self.mut_client.read_share_chunk(storage_index, 1, 0, 8) + ) + self.assertEqual((data0, data1), (b"aXYZef-0", b"abcdef-1")) + + def test_conditional_write(self): + """Uploads only happen if the test passes.""" + storage_index, write_secret, lease_secret = self.create_upload() + result_failed = self.http.result_of_with_flush( + self.mut_client.read_test_write_chunks( + storage_index, + write_secret, + lease_secret, + lease_secret, + { + 0: TestWriteVectors( + test_vectors=[TestVector(1, 4, b"FAIL")], + write_vectors=[WriteVector(offset=1, data=b"XYZ")], + ), + }, + [], + ) + ) + self.assertFalse(result_failed.success) + + # This time the test matches: + result = self.http.result_of_with_flush( + self.mut_client.read_test_write_chunks( + storage_index, + write_secret, + lease_secret, + lease_secret, + { + 0: TestWriteVectors( + test_vectors=[TestVector(1, 4, b"bcde")], + write_vectors=[WriteVector(offset=1, data=b"XYZ")], + ), + }, + [ReadVector(0, 8)], + ) + ) + self.assertTrue(result.success) + self.assertEqual( + self.http.result_of_with_flush( + self.mut_client.read_share_chunk(storage_index, 0, 0, 8) + ), + b"aXYZef-0", + ) + + def test_too_large_write(self): + """ + Writing too large of a chunk results in a REQUEST ENTITY TOO LARGE http + error. + """ + with self.assertRaises(ClientException) as e: + self.create_upload(b"0123456789" * 1024 * 1024) + self.assertEqual(e.exception.code, http.REQUEST_ENTITY_TOO_LARGE) + + def test_list_shares(self): + """``list_shares()`` returns the shares for a given storage index.""" + storage_index, _, _ = self.create_upload() + self.assertEqual( + self.http.result_of_with_flush(self.mut_client.list_shares(storage_index)), + {0, 1}, + ) + + def test_non_existent_list_shares(self): + """A non-existent storage index errors when shares are listed.""" + with self.assertRaises(ClientException) as exc: + self.http.result_of_with_flush(self.mut_client.list_shares(urandom(32))) + self.assertEqual(exc.exception.code, http.NOT_FOUND) + + def test_wrong_write_enabler(self): + """Writes with the wrong write enabler fail, and are not processed.""" + storage_index, write_secret, lease_secret = self.create_upload() + with self.assertRaises(ClientException) as exc: + self.http.result_of_with_flush( + self.mut_client.read_test_write_chunks( + storage_index, + urandom(32), + lease_secret, + lease_secret, + { + 0: TestWriteVectors( + write_vectors=[WriteVector(offset=1, data=b"XYZ")] + ), + }, + [ReadVector(0, 8)], + ) + ) + self.assertEqual(exc.exception.code, http.UNAUTHORIZED) + + # The write did not happen: + self.assertEqual( + self.http.result_of_with_flush( + self.mut_client.read_share_chunk(storage_index, 0, 0, 8) + ), + b"abcdef-0", + ) + + +class SharedImmutableMutableTestsMixin: + """ + Shared tests for mutables and immutables where the API is the same. + """ + + KIND: str # either "mutable" or "immutable" + general_client: StorageClientGeneral + client: Union[StorageClientImmutables, StorageClientMutables] + clientFactory: Callable[ + [StorageClient], Union[StorageClientImmutables, StorageClientMutables] + ] + + def upload(self, share_number: int, data_length=26) -> Tuple[bytes, bytes, bytes]: + """ + Create a share, return (storage_index, uploaded_data, lease secret). + """ + raise NotImplementedError + + def get_leases(self, storage_index: bytes) -> Iterable[LeaseInfo]: + """Get leases for the storage index.""" + raise NotImplementedError() + + def test_advise_corrupt_share(self): + """ + Advising share was corrupted succeeds from HTTP client's perspective, + and calls appropriate method on server. + """ + corrupted = [] + self.http.storage_server.advise_corrupt_share = lambda *args: corrupted.append( + args + ) + + storage_index, _, _ = self.upload(13) + reason = "OHNO \u1235" + self.http.result_of_with_flush( + self.client.advise_corrupt_share(storage_index, 13, reason) + ) + + self.assertEqual( + corrupted, + [(self.KIND.encode("ascii"), storage_index, 13, reason.encode("utf-8"))], + ) + + def test_advise_corrupt_share_unknown(self): + """ + Advising an unknown share was corrupted results in 404. + """ + storage_index, _, _ = self.upload(13) + reason = "OHNO \u1235" + self.http.result_of_with_flush( + self.client.advise_corrupt_share(storage_index, 13, reason) + ) + + for (si, share_number) in [(storage_index, 11), (urandom(16), 13)]: + with assert_fails_with_http_code(self, http.NOT_FOUND): + self.http.result_of_with_flush( + self.client.advise_corrupt_share(si, share_number, reason) + ) + + def test_lease_renew_and_add(self): + """ + It's possible the renew the lease on an uploaded mutable/immutable, by + using the same renewal secret, or add a new lease by choosing a + different renewal secret. + """ + # Create a storage index: + storage_index, _, lease_secret = self.upload(0) + + [lease] = self.get_leases(storage_index) + initial_expiration_time = lease.get_expiration_time() + + # Time passes: + self.http.clock.advance(167) + + # We renew the lease: + self.http.result_of_with_flush( + self.general_client.add_or_renew_lease( + storage_index, lease_secret, lease_secret + ) + ) + + # More time passes: + self.http.clock.advance(10) + + # We create a new lease: + lease_secret2 = urandom(32) + self.http.result_of_with_flush( + self.general_client.add_or_renew_lease( + storage_index, lease_secret2, lease_secret2 + ) + ) + + [lease1, lease2] = self.get_leases(storage_index) + self.assertEqual(lease1.get_expiration_time(), initial_expiration_time + 167) + self.assertEqual(lease2.get_expiration_time(), initial_expiration_time + 177) def test_read_of_wrong_storage_index_fails(self): """ Reading from unknown storage index results in 404. """ with assert_fails_with_http_code(self, http.NOT_FOUND): - result_of( - self.imm_client.read_share_chunk( + self.http.result_of_with_flush( + self.client.read_share_chunk( b"1" * 16, 1, 0, @@ -819,10 +1442,10 @@ class ImmutableHTTPAPITests(SyncTestCase): """ Reading from unknown storage index results in 404. """ - storage_index, _ = self.upload(1) + storage_index, _, _ = self.upload(1) with assert_fails_with_http_code(self, http.NOT_FOUND): - result_of( - self.imm_client.read_share_chunk( + self.http.result_of_with_flush( + self.client.read_share_chunk( storage_index, 7, # different share number 0, @@ -835,10 +1458,10 @@ class ImmutableHTTPAPITests(SyncTestCase): Malformed or unsupported Range headers result in 416 (requested range not satisfiable) error. """ - storage_index, _ = self.upload(1) + storage_index, _, _ = self.upload(1) def check_bad_range(bad_range_value): - client = StorageClientImmutables( + client = self.clientFactory( StorageClientWithHeadersOverride( self.http.client, {"range": bad_range_value} ) @@ -847,7 +1470,7 @@ class ImmutableHTTPAPITests(SyncTestCase): with assert_fails_with_http_code( self, http.REQUESTED_RANGE_NOT_SATISFIABLE ): - result_of( + self.http.result_of_with_flush( client.read_share_chunk( storage_index, 1, @@ -874,35 +1497,39 @@ class ImmutableHTTPAPITests(SyncTestCase): @given(data_length=st.integers(min_value=1, max_value=300000)) def test_read_with_no_range(self, data_length): """ - A read with no range returns the whole immutable. + A read with no range returns the whole mutable/immutable. """ - storage_index, uploaded_data = self.upload(1, data_length) - response = result_of( + storage_index, uploaded_data, _ = self.upload(1, data_length) + response = self.http.result_of_with_flush( self.http.client.request( "GET", self.http.client.relative_url( - "/v1/immutable/{}/1".format(_encode_si(storage_index)) + "/storage/v1/{}/{}/1".format(self.KIND, _encode_si(storage_index)) ), ) ) self.assertEqual(response.code, http.OK) - self.assertEqual(result_of(response.content()), uploaded_data) + self.assertEqual( + self.http.result_of_with_flush(response.content()), uploaded_data + ) def test_validate_content_range_response_to_read(self): """ The server responds to ranged reads with an appropriate Content-Range header. """ - storage_index, _ = self.upload(1, 26) + storage_index, _, _ = self.upload(1, 26) def check_range(requested_range, expected_response): headers = Headers() headers.setRawHeaders("range", [requested_range]) - response = result_of( + response = self.http.result_of_with_flush( self.http.client.request( "GET", self.http.client.relative_url( - "/v1/immutable/{}/1".format(_encode_si(storage_index)) + "/storage/v1/{}/{}/1".format( + self.KIND, _encode_si(storage_index) + ) ), headers=headers, ) @@ -912,232 +1539,94 @@ class ImmutableHTTPAPITests(SyncTestCase): ) check_range("bytes=0-10", "bytes 0-10/*") - # Can't go beyond the end of the immutable! - check_range("bytes=10-100", "bytes 10-25/*") + check_range("bytes=3-17", "bytes 3-17/*") + # TODO re-enable in https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3907 + # Can't go beyond the end of the mutable/immutable! + # check_range("bytes=10-100", "bytes 10-25/*") - def test_timed_out_upload_allows_reupload(self): + +class ImmutableSharedTests(SharedImmutableMutableTestsMixin, SyncTestCase): + """Shared tests, running on immutables.""" + + KIND = "immutable" + clientFactory = StorageClientImmutables + + def setUp(self): + super(ImmutableSharedTests, self).setUp() + self.http = self.useFixture(HttpTestFixture()) + self.client = self.clientFactory(self.http.client) + self.general_client = StorageClientGeneral(self.http.client) + + def upload(self, share_number, data_length=26): """ - If an in-progress upload times out, it is cancelled altogether, - allowing a new upload to occur. + Create a share, return (storage_index, uploaded_data, lease_secret). """ - self._test_abort_or_timed_out_upload_to_existing_storage_index( - lambda **kwargs: self.http.clock.advance(30 * 60 + 1) - ) - - def test_abort_upload_allows_reupload(self): - """ - If an in-progress upload is aborted, it is cancelled altogether, - allowing a new upload to occur. - """ - - def abort(storage_index, share_number, upload_secret): - return result_of( - self.imm_client.abort_upload(storage_index, share_number, upload_secret) - ) - - self._test_abort_or_timed_out_upload_to_existing_storage_index(abort) - - def _test_abort_or_timed_out_upload_to_existing_storage_index(self, cancel_upload): - """Start uploading to an existing storage index that then times out or aborts. - - Re-uploading should work. - """ - # Start an upload: - (upload_secret, _, storage_index, _) = self.create_upload({1}, 100) - result_of( - self.imm_client.write_share_chunk( - storage_index, - 1, - upload_secret, - 0, - b"123", - ) - ) - - # Now, the upload is cancelled somehow: - cancel_upload( - storage_index=storage_index, upload_secret=upload_secret, share_number=1 - ) - - # Now we can create a new share with the same storage index without - # complaint: + uploaded_data = (b"abcdefghijklmnopqrstuvwxyz" * ((data_length // 26) + 1))[ + :data_length + ] upload_secret = urandom(32) lease_secret = urandom(32) - created = result_of( - self.imm_client.create( + storage_index = urandom(16) + self.http.result_of_with_flush( + self.client.create( storage_index, - {1}, - 100, + {share_number}, + data_length, upload_secret, lease_secret, lease_secret, ) ) - self.assertEqual(created.allocated, {1}) - - # And write to it, too: - result_of( - self.imm_client.write_share_chunk( + self.http.result_of_with_flush( + self.client.write_share_chunk( storage_index, - 1, - upload_secret, - 0, - b"ABC", - ) - ) - - def test_unknown_aborts(self): - """ - Aborting uploads with an unknown storage index or share number will - result 404 HTTP response code. - """ - (upload_secret, _, storage_index, _) = self.create_upload({1}, 100) - - for si, num in [(storage_index, 3), (b"x" * 16, 1)]: - with assert_fails_with_http_code(self, http.NOT_FOUND): - result_of(self.imm_client.abort_upload(si, num, upload_secret)) - - def test_unauthorized_abort(self): - """ - An abort with the wrong key will return an unauthorized error, and will - not abort the upload. - """ - (upload_secret, _, storage_index, _) = self.create_upload({1}, 100) - - # Failed to abort becaues wrong upload secret: - with assert_fails_with_http_code(self, http.UNAUTHORIZED): - result_of( - self.imm_client.abort_upload(storage_index, 1, upload_secret + b"X") - ) - - # We can still write to it: - result_of( - self.imm_client.write_share_chunk( - storage_index, - 1, - upload_secret, - 0, - b"ABC", - ) - ) - - def test_too_late_abort(self): - """ - An abort of an already-fully-uploaded immutable will result in 405 - error and will not affect the immutable. - """ - uploaded_data = b"123" - (upload_secret, _, storage_index, _) = self.create_upload({0}, 3) - result_of( - self.imm_client.write_share_chunk( - storage_index, - 0, + share_number, upload_secret, 0, uploaded_data, ) ) + return storage_index, uploaded_data, lease_secret - # Can't abort, we finished upload: - with assert_fails_with_http_code(self, http.NOT_ALLOWED): - result_of(self.imm_client.abort_upload(storage_index, 0, upload_secret)) + def get_leases(self, storage_index): + return self.http.storage_server.get_leases(storage_index) - # Abort didn't prevent reading: - self.assertEqual( - uploaded_data, - result_of( - self.imm_client.read_share_chunk( - storage_index, - 0, - 0, - 3, - ) - ), - ) - def test_lease_renew_and_add(self): +class MutableSharedTests(SharedImmutableMutableTestsMixin, SyncTestCase): + """Shared tests, running on mutables.""" + + KIND = "mutable" + clientFactory = StorageClientMutables + + def setUp(self): + super(MutableSharedTests, self).setUp() + self.http = self.useFixture(HttpTestFixture()) + self.client = self.clientFactory(self.http.client) + self.general_client = StorageClientGeneral(self.http.client) + + def upload(self, share_number, data_length=26): """ - It's possible the renew the lease on an uploaded immutable, by using - the same renewal secret, or add a new lease by choosing a different - renewal secret. - """ - # Create immutable: - (upload_secret, lease_secret, storage_index, _) = self.create_upload({0}, 100) - result_of( - self.imm_client.write_share_chunk( - storage_index, - 0, - upload_secret, - 0, - b"A" * 100, - ) - ) - - [lease] = self.http.storage_server.get_leases(storage_index) - initial_expiration_time = lease.get_expiration_time() - - # Time passes: - self.http.clock.advance(167) - - # We renew the lease: - result_of( - self.imm_client.add_or_renew_lease( - storage_index, lease_secret, lease_secret - ) - ) - - # More time passes: - self.http.clock.advance(10) - - # We create a new lease: - lease_secret2 = urandom(32) - result_of( - self.imm_client.add_or_renew_lease( - storage_index, lease_secret2, lease_secret2 - ) - ) - - [lease1, lease2] = self.http.storage_server.get_leases(storage_index) - self.assertEqual(lease1.get_expiration_time(), initial_expiration_time + 167) - self.assertEqual(lease2.get_expiration_time(), initial_expiration_time + 177) - - def test_lease_on_unknown_storage_index(self): - """ - An attempt to renew an unknown storage index will result in a HTTP 404. + Create a share, return (storage_index, uploaded_data, lease_secret). """ + data = (b"abcdefghijklmnopqrstuvwxyz" * ((data_length // 26) + 1))[:data_length] + write_secret = urandom(32) + lease_secret = urandom(32) storage_index = urandom(16) - secret = b"A" * 32 - with assert_fails_with_http_code(self, http.NOT_FOUND): - result_of(self.imm_client.add_or_renew_lease(storage_index, secret, secret)) - - def test_advise_corrupt_share(self): - """ - Advising share was corrupted succeeds from HTTP client's perspective, - and calls appropriate method on server. - """ - corrupted = [] - self.http.storage_server.advise_corrupt_share = lambda *args: corrupted.append( - args + self.http.result_of_with_flush( + self.client.read_test_write_chunks( + storage_index, + write_secret, + lease_secret, + lease_secret, + { + share_number: TestWriteVectors( + write_vectors=[WriteVector(offset=0, data=data)] + ), + }, + [], + ) ) + return storage_index, data, lease_secret - storage_index, _ = self.upload(13) - reason = "OHNO \u1235" - result_of(self.imm_client.advise_corrupt_share(storage_index, 13, reason)) - - self.assertEqual( - corrupted, [(b"immutable", storage_index, 13, reason.encode("utf-8"))] - ) - - def test_advise_corrupt_share_unknown(self): - """ - Advising an unknown share was corrupted results in 404. - """ - storage_index, _ = self.upload(13) - reason = "OHNO \u1235" - result_of(self.imm_client.advise_corrupt_share(storage_index, 13, reason)) - - for (si, share_number) in [(storage_index, 11), (urandom(16), 13)]: - with assert_fails_with_http_code(self, http.NOT_FOUND): - result_of( - self.imm_client.advise_corrupt_share(si, share_number, reason) - ) + def get_leases(self, storage_index): + return self.http.storage_server.get_slot_leases(storage_index) diff --git a/src/allmydata/test/test_storage_https.py b/src/allmydata/test/test_storage_https.py index 3b41e8308..a11b0eed5 100644 --- a/src/allmydata/test/test_storage_https.py +++ b/src/allmydata/test/test_storage_https.py @@ -12,7 +12,7 @@ from cryptography import x509 from twisted.internet.endpoints import serverFromString from twisted.internet import reactor -from twisted.internet.task import deferLater +from twisted.internet.defer import maybeDeferred from twisted.web.server import Site from twisted.web.static import Data from twisted.web.client import Agent, HTTPConnectionPool, ResponseNeverReceived @@ -30,6 +30,7 @@ from ..storage.http_common import get_spki_hash from ..storage.http_client import _StorageClientHTTPSPolicy from ..storage.http_server import _TLSEndpointWrapper from ..util.deferredutil import async_to_deferred +from .common_system import spin_until_cleanup_done class HTTPSNurlTests(SyncTestCase): @@ -87,6 +88,10 @@ class PinningHTTPSValidation(AsyncTestCase): self.addCleanup(self._port_assigner.tearDown) return AsyncTestCase.setUp(self) + def tearDown(self): + d = maybeDeferred(AsyncTestCase.tearDown, self) + return d.addCallback(lambda _: spin_until_cleanup_done()) + @asynccontextmanager async def listen(self, private_key_path: FilePath, cert_path: FilePath): """ @@ -107,9 +112,6 @@ class PinningHTTPSValidation(AsyncTestCase): yield f"https://127.0.0.1:{listening_port.getHost().port}/" finally: await listening_port.stopListening() - # Make sure all server connections are closed :( No idea why this - # is necessary when it's not for IStorageServer HTTPS tests. - await deferLater(reactor, 0.01) def request(self, url: str, expected_certificate: x509.Certificate): """ diff --git a/src/allmydata/test/test_storage_web.py b/src/allmydata/test/test_storage_web.py index 5984b2892..b47c93849 100644 --- a/src/allmydata/test/test_storage_web.py +++ b/src/allmydata/test/test_storage_web.py @@ -161,7 +161,7 @@ class BucketCounter(unittest.TestCase, pollmixin.PollMixin): html = renderSynchronously(w) s = remove_tags(html) self.failUnlessIn(b"Total buckets: 0 (the number of", s) - self.failUnless(b"Next crawl in 59 minutes" in s or "Next crawl in 60 minutes" in s, s) + self.failUnless(b"Next crawl in 59 minutes" in s or b"Next crawl in 60 minutes" in s, s) d.addCallback(_check2) return d diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py index d859a0e00..670ac5868 100644 --- a/src/allmydata/test/test_system.py +++ b/src/allmydata/test/test_system.py @@ -117,11 +117,17 @@ class CountingDataUploadable(upload.Data): class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): - + """Foolscap integration-y tests.""" + FORCE_FOOLSCAP_FOR_STORAGE = True timeout = 180 + @property + def basedir(self): + return "system/SystemTest/{}-foolscap-{}".format( + self.id().split(".")[-1], self.FORCE_FOOLSCAP_FOR_STORAGE + ) + def test_connections(self): - self.basedir = "system/SystemTest/test_connections" d = self.set_up_nodes() self.extra_node = None d.addCallback(lambda res: self.add_extra_node(self.numclients)) @@ -149,11 +155,9 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): del test_connections def test_upload_and_download_random_key(self): - self.basedir = "system/SystemTest/test_upload_and_download_random_key" return self._test_upload_and_download(convergence=None) def test_upload_and_download_convergent(self): - self.basedir = "system/SystemTest/test_upload_and_download_convergent" return self._test_upload_and_download(convergence=b"some convergence string") def _test_upload_and_download(self, convergence): @@ -516,7 +520,6 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): def test_mutable(self): - self.basedir = "system/SystemTest/test_mutable" DATA = b"initial contents go here." # 25 bytes % 3 != 0 DATA_uploadable = MutableData(DATA) NEWDATA = b"new contents yay" @@ -746,7 +749,6 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): # plaintext_hash check. def test_filesystem(self): - self.basedir = "system/SystemTest/test_filesystem" self.data = LARGE_DATA d = self.set_up_nodes() def _new_happy_semantics(ign): @@ -1713,7 +1715,6 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): def test_filesystem_with_cli_in_subprocess(self): # We do this in a separate test so that test_filesystem doesn't skip if we can't run bin/tahoe. - self.basedir = "system/SystemTest/test_filesystem_with_cli_in_subprocess" d = self.set_up_nodes() def _new_happy_semantics(ign): for c in self.clients: @@ -1794,9 +1795,21 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase): class Connections(SystemTestMixin, unittest.TestCase): + FORCE_FOOLSCAP_FOR_STORAGE = True def test_rref(self): - self.basedir = "system/Connections/rref" + # The way the listening port is created is via + # SameProcessStreamEndpointAssigner (allmydata.test.common), which then + # makes an endpoint string parsed by AdoptedServerPort. The latter does + # dup(fd), which results in the filedescriptor staying alive _until the + # test ends_. That means that when we disown the service, we still have + # the listening port there on the OS level! Just the resulting + # connections aren't handled. So this test relies on aggressive + # timeouts in the HTTP client and presumably some equivalent in + # Foolscap, since connection refused does _not_ happen. + self.basedir = "system/Connections/rref-foolscap-{}".format( + self.FORCE_FOOLSCAP_FOR_STORAGE + ) d = self.set_up_nodes(2) def _start(ign): self.c0 = self.clients[0] @@ -1812,9 +1825,13 @@ class Connections(SystemTestMixin, unittest.TestCase): # now shut down the server d.addCallback(lambda ign: self.clients[1].disownServiceParent()) + + # kill any persistent http connections that might continue to work + d.addCallback(lambda ign: self.close_idle_http_connections()) + # and wait for the client to notice def _poll(): - return len(self.c0.storage_broker.get_connected_servers()) < 2 + return len(self.c0.storage_broker.get_connected_servers()) == 1 d.addCallback(lambda ign: self.poll(_poll)) def _down(ign): @@ -1824,3 +1841,16 @@ class Connections(SystemTestMixin, unittest.TestCase): self.assertEqual(storage_server, self.s1_storage_server) d.addCallback(_down) return d + + +class HTTPSystemTest(SystemTest): + """HTTP storage protocol variant of the system tests.""" + + FORCE_FOOLSCAP_FOR_STORAGE = False + + + +class HTTPConnections(Connections): + """HTTP storage protocol variant of the connections tests.""" + FORCE_FOOLSCAP_FOR_STORAGE = False + diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index 527b235bd..3715d1aca 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -46,9 +46,10 @@ from hypothesis.strategies import ( binary, ) -from testtools import ( - TestCase, +from .common import ( + SyncTestCase, ) + from testtools.matchers import ( Always, Equals, @@ -61,7 +62,7 @@ from testtools.twistedsupport import ( ) -class FakeWebTest(TestCase): +class FakeWebTest(SyncTestCase): """ Test the WebUI verified-fakes infrastucture """ diff --git a/src/allmydata/test/test_upload.py b/src/allmydata/test/test_upload.py index 8d5435e88..18192de6c 100644 --- a/src/allmydata/test/test_upload.py +++ b/src/allmydata/test/test_upload.py @@ -983,7 +983,7 @@ class EncodingParameters(GridTestMixin, unittest.TestCase, SetDEPMixin, num_segments = encoder.get_param("num_segments") d = selector.get_shareholders(broker, sh, storage_index, share_size, block_size, num_segments, - 10, 3, 4) + 10, 3, 4, encoder.get_uri_extension_size()) def _have_shareholders(upload_trackers_and_already_servers): (upload_trackers, already_servers) = upload_trackers_and_already_servers assert servers_to_break <= len(upload_trackers) diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py new file mode 100644 index 000000000..f12c201d1 --- /dev/null +++ b/src/allmydata/util/pid.py @@ -0,0 +1,120 @@ +import psutil + +# the docs are a little misleading, but this is either WindowsFileLock +# or UnixFileLock depending upon the platform we're currently on +from filelock import FileLock, Timeout + + +class ProcessInTheWay(Exception): + """ + our pidfile points at a running process + """ + + +class InvalidPidFile(Exception): + """ + our pidfile isn't well-formed + """ + + +class CannotRemovePidFile(Exception): + """ + something went wrong removing the pidfile + """ + + +def _pidfile_to_lockpath(pidfile): + """ + internal helper. + :returns FilePath: a path to use for file-locking the given pidfile + """ + return pidfile.sibling("{}.lock".format(pidfile.basename())) + + +def parse_pidfile(pidfile): + """ + :param FilePath pidfile: + :returns tuple: 2-tuple of pid, creation-time as int, float + :raises InvalidPidFile: on error + """ + with pidfile.open("r") as f: + content = f.read().decode("utf8").strip() + try: + pid, starttime = content.split() + pid = int(pid) + starttime = float(starttime) + except ValueError: + raise InvalidPidFile( + "found invalid PID file in {}".format( + pidfile + ) + ) + return pid, starttime + + +def check_pid_process(pidfile): + """ + If another instance appears to be running already, raise an + exception. Otherwise, write our PID + start time to the pidfile + and arrange to delete it upon exit. + + :param FilePath pidfile: the file to read/write our PID from. + + :raises ProcessInTheWay: if a running process exists at our PID + """ + lock_path = _pidfile_to_lockpath(pidfile) + + try: + # a short timeout is fine, this lock should only be active + # while someone is reading or deleting the pidfile .. and + # facilitates testing the locking itself. + with FileLock(lock_path.path, timeout=2): + # check if we have another instance running already + if pidfile.exists(): + pid, starttime = parse_pidfile(pidfile) + try: + # if any other process is running at that PID, let the + # user decide if this is another legitimate + # instance. Automated programs may use the start-time to + # help decide this (if the PID is merely recycled, the + # start-time won't match). + psutil.Process(pid) + raise ProcessInTheWay( + "A process is already running as PID {}".format(pid) + ) + except psutil.NoSuchProcess: + print( + "'{pidpath}' refers to {pid} that isn't running".format( + pidpath=pidfile.path, + pid=pid, + ) + ) + # nothing is running at that PID so it must be a stale file + pidfile.remove() + + # write our PID + start-time to the pid-file + proc = psutil.Process() + with pidfile.open("w") as f: + f.write("{} {}\n".format(proc.pid, proc.create_time()).encode("utf8")) + except Timeout: + raise ProcessInTheWay( + "Another process is still locking {}".format(pidfile.path) + ) + + +def cleanup_pidfile(pidfile): + """ + Remove the pidfile specified (respecting locks). If anything at + all goes wrong, `CannotRemovePidFile` is raised. + """ + lock_path = _pidfile_to_lockpath(pidfile) + with FileLock(lock_path.path): + try: + pidfile.remove() + except Exception as e: + raise CannotRemovePidFile( + "Couldn't remove '{pidfile}': {err}.".format( + pidfile=pidfile.path, + err=e, + ) + ) diff --git a/tox.ini b/tox.ini index 859cf18e0..fc95a0469 100644 --- a/tox.ini +++ b/tox.ini @@ -97,7 +97,7 @@ setenv = COVERAGE_PROCESS_START=.coveragerc commands = # NOTE: 'run with "py.test --keep-tempdir -s -v integration/" to debug failures' - py.test --timeout=1800 --coverage -v {posargs:integration} + py.test --timeout=1800 --coverage -s -v {posargs:integration} coverage combine coverage report