| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| | |
|
| |
|
|
|
|
|
| |
Promote the associated comments.
As suggested here:
https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/2560#note_3097188
|
| |
|
|
|
|
|
|
|
| |
The DataStream is sometimes disassembled, eg by split. When that
happens, the StreamAccount would be dropped - and that was the only
strong reference.
Put a StreamAccount in each of the pieces, instead of just in the
combined DataStream struct.
|
| |
|
|
|
|
|
|
|
| |
We need the mq account for the stream not to collapse. The
ResolveStream object needs to contain a strong reference to it.
Have begin_stream_impl return the StreamAccount, rather than taking it
as a parameter. That makes this bug a little more obvious. It also
centralises the StreamAccount creation.
|
| | |
|
| | |
|
| | |
|
| |
|
|
| |
This will make it easier to change their types.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Having this in the `tor-async-utils` crate prevents us from doing both
of the following without introducing a circular dependency:
* using it in `tor-rtmock` (which we currently do, particularly in
tests).
* using `tor-rtmock` to test things in `tor-async-utils`. We don't do
this yet, but it is generally sensible to do so. In particular we
want to move the `stream_peak` module there, which is currently tested
with `tor-rtmock`.
Moving this into its own crate avoids this circular dependency.
|
| |
|
|
|
| |
From
<https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/2340#note_3062531>
|
| |
|
|
|
|
| |
Encapsulate flow-control into a separate object that partially abstracts
away the difference between window-based (legacy) flow control and
xon-based (prop324) flow control.
|
| | |
|
| | |
|
| |
|
|
|
|
|
| |
In particular, clarify that dropping the DataWriter on its own does
nothing unless the DataReader is also dropped.
Related to #1368.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously we had a bug where `<DataWriter as AsyncWrite>::close`
(or `shutdown` in tokio-land) would not actually have any effect.
It _would_ drop the `StreamTarget` held by the `DataWriter`, but
since the `DataReader` also held a `StreamTarget`, the
MPSC channel would not get closed, and the circuit reactor would
not realize that the stream wanted to shut down.
Now we use `mpsc::Sender::close_channel` to make our closes
effectual.
Closes #1368.
Additionally, we fix a bug where `poll_close()` never actually did
anything if the buffer had nothing in it when it was called.
Previously, `poll_flush_impl()` would exit immediately if it had no
data to flush. That isn't what we want when we are closing!
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
No actual bug here, just technical debt:
For `SendWindow`s, our tag system already ensured that we rejected
any SENDME that didn't correspond to an appropriate drain. Still,
it doesn't hurt to check.
For `RecvWindow`s, it would have been a protocol violation if we
ever did this, but it makes sense to make it an internal error if we
try.
Part of #1383.
|
| | |
|
| | |
|
| |
|
|
| |
(Doing this to prevent us having two structs with the same name.)
|
| |
|
|
|
|
|
| |
Based on designs in #1124.
Note that there is a TODO here about a hack I had to do to appease
the borrow checker.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
| |
Prop 340:
https://spec.torproject.org/proposals/340-packed-and-fragmented.html
This updates the decoding API to support multiple versions of the relay
cell encoding, including the new encoding proposed in prop340 that
supports relay message packing and fragmentation.
This commit doesn't actually add support for that new encoding yet.
|
| |
|
|
|
| |
For consistency with the terminology proposed in
https://gitlab.torproject.org/tpo/core/torspec/-/issues/253
|
| |
|
|
|
|
|
| |
We never actually constructed these before, but now we enforce it at
the API level.
Part of #1269.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit is pure renaming, done automatically with rust-analyzer.
Comment fixes and other cleanups will be in the subsequent commits.
We're doing this renaming because we need a name for
the combination of a `RelayMsg` and an `Option<StreamId>`
that we use when we have a `RelayMsg`
we intend to route to a given stream or circuit internally.
Previously we called this a `RelayCell`,
but that name was already somewhat inaccurate,
and will become _very_ inaccurate with the arrival of prop340,
which breaksthe 1:1 relationship between relay cells
and relay messages.
(If we didn't do this renaming now, we'd soon be making
the relationship between `UnparsedRelayCell`and `RelayCell`
many-to-many, which would be ridiculous and confusing.)
The `RelayMsgOuter` name is a placeholder:
We expect that we'll want to rename this type,
and may also want to rename `RelayMsg`,
and unify our vocabulary in other areas too.
But such a renaming will have to wait
for a larger discussion affecting the specifications,
so that we can use the same vocabulary everywhere.
|
| |
|
|
|
|
|
|
|
| |
While looking for differences, we found that C tor always
omits the flags and the hostname from a BEGIN message sent on an
onion service circuit. In torspec!179, we specified that behavior.
This patch brings arti into conformance.
Closes #1077.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Because dropping a `StreamTarget` causes the circuit reactor to send
an End, the previous do-nothing implementation of `discard()` wasn't
sufficient to cause the request to be ignored without sending an
End.
This commit modifies our "close pending stream" behavior to only
optionally send an End message. To avoid confusion, I'm using a new
`CloseStreamBehavior` enum rather than an `Option<End>`, since we
had previously used `None` in some cases to indicate a default
(misc) end message.
|
| | |
|
| |
|
|
|
|
| |
Now that every state-change function consumes the IncomingStream,
there is no longer any reason to keep track of an internal state
enum.
|
| | |
|
| |
|
|
|
| |
Now that IncomingStream doesn't implement Drop, we can just
destructure it.
|
| |
|
|
|
| |
Since dropping a StreamTarget will send an End, we don't need
to call reject_internal in this case.
|
| |
|
|
|
|
|
|
|
| |
It turns out that we can make `IncomingStream::reject()` consume
self, thus making it impossible to hit the double-close error
from outside the `tor-proto` crate.
Also, we rename `StreamTarget::close()` to `close_pending()` to
better reflect its limited applicability.
|
| |
|
|
|
| |
This should be smaller and faster than vec, and save some
allocations. It could also avoid a tiny sidechannel.
|
| | |
|
| |
|
|
| |
cargo fmt, precisely.
|
| | |
|
| |
|
|
|
|
|
|
| |
A warning is getting in my way when I run
cargo clippy -p tor-hsservice --all-features
See also
https://gitlab.torproject.org/tpo/core/arti/-/issues/1006#note_2932088
|
| |
|
|
| |
These functions only ever return `Bug` errors.
|
| | |
|
| |
|
|
|
|
|
|
|
|
| |
This commit introduces an `IncomingStreamState` enum, which indicates
whether the stream was accepted, discarded, or rejected, or if it is
still pending. The `is_rejected`/`is_accepted` boolean flags are no
longer needed.
Without this change, we'd need to introduce yet another boolean flag
when we implement `discard()` (for the "discarded" state).
|
| |\
| |
| |
| |
| | |
tor-proto: Replace IncomingStreamMsg with IncomingStreamRequest.
See merge request tpo/core/arti!1477
|
| | |
| |
| |
| |
| |
| |
| |
| | |
The two enums essentially serve the same purpose, so we don't
need both of them.
This also addresses the TODO that says we should return an error if
`accept_data` is called for a RESOLVE stream.
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
blocking.
Instead of having 2 version of `StreamTarget::close` (a blocking one and
a nonblocking one), we can just return the `oneshot::Receiver` for
receiving the reactor's response and let the caller of
`StreamTarget::close` decide whether to block.
This allows us to reduce some code duplication in the `IncomingStream`
implementation.
|
| |/ |
|
| | |
|
| |
|
|
|
| |
The behaviour of `IncomingStream::drop` is going to depend on whether
the stream was accepted or not.
|
