This removes the last uses of unsafe `Pin` functions in actix-web.
This PR adds a `Pin<Box<_>>` wrapper to `DispatcherState::Upgrade`,
`State::ExpectCall`, and `State::ServiceCall`.
The previous uses of the futures `State::ExpectCall` and `State::ServiceCall`
were Undefined Behavior - a future was obtained from `self.expect.call`
or `self.service.call`, pinned on the stack, and then immediately
returned from `handle_request`. The only alternative to using `Box::pin`
would be to refactor `handle_request` to write the futures directly into
their final location, or avoid polling them before they are returned.
The previous use of `DispatcherState::Upgrade` doesn't seem to be
unsound. However, having data pinned inside an enum that we
`std::mem::replace` would require some careful `unsafe` code to ensure
that we never call `std::mem::replace` when the active variant contains
pinned data. By using `Box::pin`, we any possibility of future
refactoring accidentally introducing undefined behavior.
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
Fixes#1321
A better fix would be to change `MessageBody` to take a `Pin<&mut
Self>`, rather than a `Pin<&mut Self>`. This will avoid requiring the
use of `Box` for all consumers by allowing the caller to determine how
to pin the `MessageBody` implementation (e.g. via stack pinning).
However, doing so is a breaking change that will affect every user of
`MessageBody`. By pinning the inner stream ourselves, we can fix the
undefined behavior without breaking the API.
I've included @sebzim4500's reproduction case as a new test case.
However, due to the nature of undefined behavior, this could pass (and
not segfault) even if underlying issue were to regress.
Unfortunately, until rust-lang/unsafe-code-guidelines#148 is resolved,
it's not even possible to write a Miri test that will pass when the bug
is fixed.
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
* Enforce safety of `downcast_ref` at compile time.
The safety of `downcast_ref` requires that `__private_get_type_id__` not
be overriden by callers, since the returned `TypeId` is used to check if
the cast is safe. However, all trait methods in Rust are public, so
users can override `__private_get_type_id__` despite it being
`#[doc(hidden)]`.
This commit makes `__private_get_type_id__` return a type with a private
constructor, ensuring that the only possible implementation is the
default implementation. A more detailed explanation is provided in the
comments added to the file.
Note that the standard library was affected by this type of issue with
the `Error::type_id` function: see https://blog.rust-lang.org/2019/05/14/Rust-1.34.2.html#whats-in-1.34.2-stable
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
* Replace `UnsafeCell` with `Cell` in `DateServiceInner`
This ensures that it's impossible to cause undefined behavior by
accidentally violating Rust's aliasing rules (e.g. passing a closure to
`set_date` which ends up invoking `reset` or `update` on the inner
`DateServiceInner`).
There might be a tiny amount of overhead from copying the `Option<(Date,
Instant)>` rather than taking a reference, but it shouldn't be
measurable.
Since the wrapped type is `Copy`, a `Cell` can be used, avoiding the
runtime overhead of a `RefCell`.
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
* Use `OffsetDateTime` instead of `PrimitiveDateTime`
* Parse time strings with `PrimitiveDateTime::parse` instead of `OffsetDateTime::parse`
* Remove unused `time` dependency from actix-multipart
* Fix a few errors with time related tests from the `time` upgrade
* Implement logic to convert a RFC 850 two-digit year into a full length year, and organize time parsing related functions
* Upgrade `time` to 0.2.2
* Correctly parse C's asctime time format using time 0.2's new format patterns
* Update CHANGES.md
* Use `time` without any of its deprecated functions
* Enforce a UTC time offset when converting an `OffsetDateTime` into a Header value
* Use the more readable version of `Duration::seconds(0)`, `Duration::zero()`
* Remove unneeded conversion of time::Duration to std::time::Duration
* Use `OffsetDateTime::as_seconds_f64` instead of manually calculating the amount of seconds from nanoseconds
* Replace a few additional instances of `Duration::seconds(0)` with `Duration::zero()`
* Truncate any nanoseconds from a supplied `Duration` within `Cookie::set_max_age` to ensure two Cookies with the same amount whole seconds equate to one another
* Fix the actix-http:🍪:do_not_panic_on_large_max_ages test
* Convert `Cookie::max_age` and `Cookie::expires` examples to `time` 0.2
Mainly minor changes. Type inference can be used alongside the new
`time::parse` method, such that the type doesn't need to be specified.
This will be useful if a refactoring takes place that changes the type.
There are also new macros, which are used where possible.
One change that is not immediately obvious, in `HttpDate`, there was an
unnecessary conditional. As the time crate allows for negative durations
(and can perform arithmetic with such), the if/else can be removed
entirely.
Time v0.2.3 also has some bug fixes, which is why I am not using a more
general v0.2 in Cargo.toml.
v0.2.3 has been yanked, as it was backwards imcompatible. This version
reverts the breaking change, while still supporting rustc back to
1.34.0.
* Add missing `time::offset` macro import
* Fix type confusion when using `time::parse` followed by `using_offset`
* Update `time` to 0.2.5
* Update CHANGES.md
Co-authored-by: Jacob Pratt <the.z.cuber@gmail.com>
* Skip empty chucks for BodyStream and SizedStream when streaming response (#1267)
* Fix tests to fail on previous implementation
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
Most of the relevant struct already had a `#[pin_project]` attribute,
but it wasn't being used.
The remaining uses of `Pin::new_unchecked` all involve going from a
`&mut T` to a `Pin<&mut T>`, without directly observing a `Pin<&mut T>`
first. As such, they cannot be replaced by `pin_project`
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
* Replace sha1 dependency with sha-1
This other crate is being maintained, and it offers better performances
when using the `asm` feature (especially [on
AArch64](https://github.com/RustCrypto/hashes/pull/97)).
* Update CHANGES.md with the sha-1 migration
* Add a test for hash_key()
* Fix filename encoding in Content-Disposition of acitx_files::NamedFile
* Add more comments on how to use Content-Disposition header properly & Fix some trivial problems
* Improve Content-Disposition filename(*) parameters of actix_files::NamedFile
* Tweak Content-Disposition parse to accept empty param value in quoted-string
* Fix typos in comments in .../content_disposition.rs (pointed out by @JohnTitor)
* Update CHANGES.md
* Update CHANGES.md again
* Let ResponseError render w/ 'text/plain; charset=utf-8' header (#1118)
Trait ResponseError originally render Error messages with header
`text/plain` , which causes browsers (i.e. Firefox 70.0) with
Non-English locale unable to render UTF-8 responses with non-English
characters correctly. i.e. emoji.
This fix solved this problem by specifying the charset of `text/plain`
as utf-8, which is the default charset in rust.
Before actix-web consider to support other charsets, this hotfix is
enough.
Test case:
fn test() -> Result<String, actix_web::Error> {
Err(actix_web::error::ErrorForbidden("ðtest"))
}
* Update actix-http/CHANGES.md for #1118
* Expose ContentDisposition in actix-multipart to fix broken doc link
* Revert "Expose ContentDisposition in actix-multipart to fix broken doc link"
This reverts commit e90d71d16c.
* Unhide actix-http::header::common docs
These types are used in other exported documented interfaces and create
broken links if not documented.
See `actix_multipart::Field.content_disposition`
* Initial commit
* Added extra_headers
* Added freeze() method to ClientRequest which produces a 'read-only' copy of a request suitable for retrying the send operation
* Additional methods for FrozenClientRequest
* Fix
* Increased crates versions
* Fixed a unit test. Added one more unit test.
* Added RequestHeaderWrapper
* Small fixes
* Renamed RequestHeadWrapper->RequestHeadType
* Updated CHANGES.md files
* Small fix
* Small changes
* Removed *_extra methods from Connection trait
* Added FrozenSendBuilder
* Added FrozenSendBuilder
* Minor fix
* Replaced impl Future with concrete Future implementation
* Small renaming
* Renamed Send->SendBody
* add rustls support for actix-http and awc
* fix features conflict
* remove unnecessary duplication
* test server with rust-tls
* fix
* test rustls
* awc rustls test
* format
* tests
* fix dependencies
* fixes and add changes
* remove test-server and Cargo.toml dev-dependencies changes
* cargo fmt
* Log error results in Logger middleware (closes#938)
* Log internal server errors with an ERROR log level
* Logger middleware: don't log 500 internal server errors, as Actix now logs them always
* Changelog
* Replace UnsafeCell in DateServiceInner with Cell
The previous API was extremely dangerous - calling `get_ref()`
followed by `reset()` would trigger instant UB, without requiring
any `unsafe` blocks in the caller.
By making DateInner `Copy`, we can use a normal `Cell` instead
of an `UnsafeCell`. This makes it impossible to cause UB (or even panic)
with the API.
* Split unsafe block HttpServiceHandlerResponse
Also add explanation of the safety of the usage of `unsafe`
* Replace UnsafeCell with RefCell in PayloadRef
This ensures that a mistake in the usage of 'get_mut' will cause
a panic, not undefined behavior.
* Expose the max limit for payload sizes in Websocket Actors.
* Revert to previous not-formatted code.
* Implement WebsocketContext::with_codec and make Codec Copy and Clone.
* Fix formatting.
* Fix formatting.
* awc: Fix typo on ResponseError documentation
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
* http: Fix typo on ResponseError documentation
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
* http: Expand type names for openssl related errors documentation
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
* Support to set header names of `ClientRequest` as Camel-Case
This is the case for supporting to request for servers which don't
perfectly implement the `RFC 7230`. It is important for an app
which uses `ClientRequest` as core part.
* Add field `upper_camel_case_headers` to `ClientRequest`.
* Add function `set_upper_camel_case_headers` to `ClientRequest`
and `ClientRequestBuilder` to set field `upper_camel_case_headers`.
* Add trait `client::writer::UpperCamelCaseHeader` for
`http::header::HeaderName`, let it can be converted to Camel-Case
then writed to buffer.
* Add test `test_client::test_upper_camel_case_headers`.
* Support upper Camel-Case headers
* [actix-http] Add field `upper_camel_case_headers` for `RequestHead`
* [actix-http] Add code for `MessageType` to support upper camel case
* [awc] Add functions for `ClientRequest` to set upper camel case
* Use `Flags::CAMEL_CASE` for upper camel case of headers
