Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no_std support for the url crate #831

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
35653b0
Make form_urlencoded no_std compatible
madsmtm Feb 17, 2022
194122c
Make data-url no_std compatible
madsmtm Aug 12, 2021
422a260
Make idna no_std compatible
madsmtm Feb 17, 2022
81ee074
Test no_std support
madsmtm Oct 5, 2021
67d2a4c
Initial work at making url no_std compatible
madsmtm Aug 12, 2021
db92dd2
Merge branch 'master' into no_std
domenukk Apr 7, 2023
bb7955a
Fixed testcases for no_std
domenukk Apr 8, 2023
a98aef8
fix no_std for nightly
domenukk Apr 8, 2023
0d14d5a
Add no_std to CI
domenukk Apr 8, 2023
315eb43
Fix no_std for deps
domenukk Apr 10, 2023
6d622ab
Moved to no-std-net
domenukk Apr 10, 2023
3efe254
Added unstable, no_std_net options
domenukk Apr 16, 2023
36bc5b3
Make CI happy by removing no-std-net/serde (not supported in Cargo.to…
domenukk Apr 27, 2023
29bfcf7
Merge branch 'master' into no_std
domenukk Apr 27, 2023
e16eb2b
Only test aarch64 on ubuntu
domenukk May 9, 2023
3dd87d8
Fix CI some more
domenukk May 11, 2023
af1d5d5
Windows fix
domenukk Jun 6, 2023
b908f7f
Merge branch 'master' into no_std
domenukk Jun 6, 2023
fd98554
Merge branch 'master' into no_std
domenukk Jul 12, 2023
b0bb942
Merge branch 'master' into no_std
domenukk Jul 12, 2023
047d334
Merge branch 'master' into no_std
lucacasonato Jul 14, 2023
028424a
Merge branch 'master' into no_std
domenukk Jul 16, 2023
7073957
Only checking crate for no_std
domenukk Jul 16, 2023
be123af
only build no_std aarch64 none on nightly
domenukk Jul 19, 2023
c145584
Only build for aarch64 on nightly
domenukk Jul 19, 2023
be73851
Merge branch 'master' into no_std
domenukk Jul 19, 2023
96e2a04
Merge branch 'master' into no_std
domenukk Nov 23, 2023
268c17e
Fix test
domenukk Nov 23, 2023
0c02cd9
fmt
domenukk Nov 23, 2023
316c868
Merge branch 'master' into no_std
domenukk Nov 28, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ jobs:
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}
# Add toolchain for no_std tests
- run: rustup target add aarch64-unknown-none
- run: cargo build --all-targets
# Run tests
- name: Run tests
Expand All @@ -49,6 +51,8 @@ jobs:
run: cargo test --test debugger_visualizer --features "url/serde,url/debugger_visualizer" -- --test-threads=1
- name: Test `no_std` support
run: cargo test --no-default-features --features=alloc
- name: Build `aarch64-unknown-none` with `no_std`
run: cargo +nightly build -Zbuild-std=core,alloc --target aarch64-unknown-none -v --release

WASM:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion idna/src/uts46.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ impl Idna {
return Errors::default();
}
let mut errors = processing(domain, self.config, &mut self.normalized, out);
self.output = std::mem::replace(out, String::with_capacity(out.len()));
self.output = core::mem::replace(out, String::with_capacity(out.len()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, this fixes a bug in the current idna no_std support. Why is it not caught in CI, and should I open an extra PR for it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recently dabbled with no_std stuff as well and I think the sure-fire way to test no_std compatibility is to:

  • target a platform that does not have std (certain ARM platforms) OR
  • have a really simple no_std binary crate which uses the libraries

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I'm trying it with cargo +nightly build -Zbuild-std=core,alloc --target aarch64-unknown-none -v --release --no-default-features

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you split this off into a separate PR?

let mut first = true;
for label in self.output.split('.') {
if !first {
Expand Down
10 changes: 6 additions & 4 deletions url/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ debugger_test = "0.1"
debugger_test_parser = "0.1"

[dependencies]
form_urlencoded = { version = "1.1.0", path = "../form_urlencoded" }
idna = { version = "0.3.0", path = "../idna" }
percent-encoding = { version = "2.2.0", path = "../percent_encoding" }
form_urlencoded = { version = "1.1.0", path = "../form_urlencoded", default-features = false }
idna = { version = "0.3.0", path = "../idna", default-features = false }
percent-encoding = { version = "2.2.0", path = "../percent_encoding", default-features = false }
serde = {version = "1.0", optional = true, features = ["derive"]}

[features]
default = []
default = ["std"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fear this might be a fairly big breaking change?

When I added no_std support for the other libraries in this crate, it was more trivial, since there was only three crates that disabled default features - but that is not the case for url; there are many crates specifying default-features = false, so even though there was no default features previously, this change will affect all of those crates.

Python script for finding crates using `default-features = false`.
import json
import requests
import sys

API_URL = "https://crates.io/api/v1/crates"

crate_name = sys.argv[1]

page = 1
while True:
    # print(f"page {page}")
    data = json.loads(requests.get(f"{API_URL}/{crate_name}/reverse_dependencies?per_page=100&page={page}").content)
    page += 1

    if len(data['versions']) == 0:
        break

    versions = {x['id']: x for x in data['versions']}

    for dep in data['dependencies']:
        if not dep['default_features']:
            crate = versions[dep['version_id']]
            print(f"{crate['crate']} had `default-features = false`")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any other way?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... I see two:

  • Bundling this in with other breaking changes, and releasing url v0.3
  • Calling the feature unstable-no-std instead, to signify that it may break other crates in the dependency chain, and that it is a bad way of doing it which will be rectified once v0.3 is released.

std = ["idna/std", "percent-encoding/std", "form_urlencoded/std", "alloc"]
alloc = []
# UNSTABLE FEATURES (requires Rust nightly)
# Enable to use the #[debugger_visualizer] attribute.
debugger_visualizer = []
Expand Down
14 changes: 12 additions & 2 deletions url/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,18 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::cmp;
use std::fmt::{self, Formatter};
use alloc::{
borrow::ToOwned,
string::{String, ToString},
vec::Vec,
};
#[cfg(not(feature = "std"))]
use core::net::{Ipv4Addr, Ipv6Addr};
use core::{
cmp,
fmt::{self, Formatter},
};
#[cfg(feature = "std")]
use std::net::{Ipv4Addr, Ipv6Addr};

use percent_encoding::{percent_decode, utf8_percent_encode, CONTROLS};
Expand Down
141 changes: 113 additions & 28 deletions url/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,27 +127,47 @@ url = { version = "2", features = ["serde"] }
feature(debugger_visualizer),
debugger_visualizer(natvis_file = "../../debug_metadata/url.natvis")
)]
#![no_std]
#![cfg_attr(not(feature = "std"), feature(ip_in_core))]

pub use form_urlencoded;

// For forwards compatibility
#[cfg(feature = "std")]
extern crate std;

#[macro_use]
extern crate alloc;

#[cfg(not(feature = "alloc"))]
compile_error!("the `alloc` feature must be enabled");

#[cfg(feature = "serde")]
extern crate serde;

use crate::host::HostInternal;
use crate::parser::{to_u32, Context, Parser, SchemeType, PATH_SEGMENT, USERINFO};
use percent_encoding::{percent_decode, percent_encode, utf8_percent_encode};
use std::borrow::Borrow;
use std::cmp;
use std::fmt::{self, Write};
use std::hash;
use std::io;
use std::mem;
use std::net::{IpAddr, SocketAddr, ToSocketAddrs};
use std::ops::{Range, RangeFrom, RangeTo};
use std::path::{Path, PathBuf};
use std::str;

use std::convert::TryFrom;
use crate::parser::{to_u32, Context, Parser, SchemeType, USERINFO};
use alloc::borrow::ToOwned;
use alloc::string::{String, ToString};
use core::borrow::Borrow;
use core::cmp;
use core::convert::TryFrom;
use core::fmt::{self, Write};
use core::hash;
use core::mem;
#[cfg(not(feature = "std"))]
use core::net::IpAddr;
use core::ops::{Range, RangeFrom, RangeTo};
use core::str;
use percent_encoding::utf8_percent_encode;
#[cfg(feature = "std")]
use std::net::IpAddr;
#[cfg(feature = "std")]
use std::{
io,
net::{SocketAddr, ToSocketAddrs},
path::{Path, PathBuf},
};

pub use crate::host::Host;
pub use crate::origin::{OpaqueOrigin, Origin};
Expand Down Expand Up @@ -1237,10 +1257,11 @@ impl Url {
/// })
/// }
/// ```
#[cfg(feature = "std")]
pub fn socket_addrs(
&self,
default_port_number: impl Fn() -> Option<u16>,
) -> io::Result<Vec<SocketAddr>> {
) -> io::Result<alloc::vec::Vec<SocketAddr>> {
// Note: trying to avoid the Vec allocation by returning `impl AsRef<[SocketAddr]>`
// causes borrowck issues because the return value borrows `default_port_number`:
//
Expand All @@ -1249,6 +1270,7 @@ impl Url {
// > This RFC proposes that *all* type parameters are considered in scope
// > for `impl Trait` in return position

// TODO: Return custom error type to support no_std
fn io_result<T>(opt: Option<T>, message: &str) -> io::Result<T> {
opt.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, message))
}
Expand Down Expand Up @@ -1310,9 +1332,23 @@ impl Url {
///
/// ```
/// use url::Url;
/// # use std::error::Error;
///
/// # fn run() -> Result<(), Box<dyn Error>> {
/// # use url::ParseError;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not super pretty. If URL had a no_std error type that also supports strings, it could be nicer. However, it's not visible in docs so should be fine

/// # #[derive(Debug)]
/// # /// A simple wrapper error struct for `no_std` support
/// # struct TestError;
/// # impl From<ParseError> for TestError {
/// # fn from(value: ParseError) -> Self {
/// # TestError {}
/// # }
/// # }
/// # impl From<&str> for TestError {
/// # fn from(value: &str) -> Self {
/// # TestError {}
/// # }
/// # }
///
/// # fn run() -> Result<(), TestError> {
/// let url = Url::parse("https://example.com/foo/bar")?;
/// let mut path_segments = url.path_segments().ok_or_else(|| "cannot be base")?;
/// assert_eq!(path_segments.next(), Some("foo"));
Expand Down Expand Up @@ -1717,9 +1753,22 @@ impl Url {
///
/// ```
/// use url::Url;
/// # use std::error::Error;
/// # use url::ParseError;
/// # #[derive(Debug)]
/// # /// A simple wrapper error struct for `no_std` support
/// # struct TestError;
/// # impl From<ParseError> for TestError {
/// # fn from(value: ParseError) -> Self {
/// # TestError {}
/// # }
/// # }
/// # impl From<&str> for TestError {
/// # fn from(value: &str) -> Self {
/// # TestError {}
/// # }
/// # }
///
/// # fn run() -> Result<(), Box<dyn Error>> {
/// # fn run() -> Result<(), TestError> {
/// let mut url = Url::parse("ssh://example.net:2048/")?;
///
/// url.set_port(Some(4096)).map_err(|_| "cannot be base")?;
Expand All @@ -1736,9 +1785,22 @@ impl Url {
///
/// ```rust
/// use url::Url;
/// # use std::error::Error;
/// # use url::ParseError;
/// # #[derive(Debug)]
/// # /// A simple wrapper error struct for `no_std` support
/// # struct TestError;
/// # impl From<ParseError> for TestError {
/// # fn from(value: ParseError) -> Self {
/// # TestError {}
/// # }
/// # }
/// # impl From<&str> for TestError {
/// # fn from(value: &str) -> Self {
/// # TestError {}
/// # }
/// # }
///
/// # fn run() -> Result<(), Box<dyn Error>> {
/// # fn run() -> Result<(), TestError> {
/// let mut url = Url::parse("https://example.org/")?;
///
/// url.set_port(Some(443)).map_err(|_| "cannot be base")?;
Expand Down Expand Up @@ -2419,7 +2481,12 @@ impl Url {
/// # run().unwrap();
/// # }
/// ```
#[cfg(any(unix, windows, target_os = "redox", target_os = "wasi"))]
///
/// This method is only available if the `std` Cargo feature is enabled.
#[cfg(all(
feature = "std",
any(unix, windows, target_os = "redox", target_os = "wasi")
))]
#[allow(clippy::result_unit_err)]
pub fn from_file_path<P: AsRef<Path>>(path: P) -> Result<Url, ()> {
let mut serialization = "file://".to_owned();
Expand Down Expand Up @@ -2456,7 +2523,12 @@ impl Url {
///
/// Note that `std::path` does not consider trailing slashes significant
/// and usually does not include them (e.g. in `Path::parent()`).
#[cfg(any(unix, windows, target_os = "redox", target_os = "wasi"))]
///
/// This method is only available if the `std` Cargo feature is enabled.
#[cfg(all(
feature = "std",
any(unix, windows, target_os = "redox", target_os = "wasi")
))]
#[allow(clippy::result_unit_err)]
pub fn from_directory_path<P: AsRef<Path>>(path: P) -> Result<Url, ()> {
let mut url = Url::from_file_path(path)?;
Expand Down Expand Up @@ -2572,8 +2644,13 @@ impl Url {
/// or if `Path::new_opt()` returns `None`.
/// (That is, if the percent-decoded path contains a NUL byte or,
/// for a Windows path, is not UTF-8.)
///
/// This method is only available if the `std` Cargo feature is enabled.
#[inline]
#[cfg(any(unix, windows, target_os = "redox", target_os = "wasi"))]
#[cfg(all(
feature = "std",
any(unix, windows, target_os = "redox", target_os = "wasi")
))]
#[allow(clippy::result_unit_err)]
pub fn to_file_path(&self) -> Result<PathBuf, ()> {
if let Some(segments) = self.path_segments() {
Expand Down Expand Up @@ -2777,11 +2854,13 @@ impl<'de> serde::Deserialize<'de> for Url {
}
}

#[cfg(any(unix, target_os = "redox", target_os = "wasi"))]
#[cfg(all(feature = "std", any(unix, target_os = "redox", target_os = "wasi")))]
fn path_to_file_url_segments(
path: &Path,
serialization: &mut String,
) -> Result<(u32, HostInternal), ()> {
use crate::parser::PATH_SEGMENT;
use percent_encoding::percent_encode;
#[cfg(any(unix, target_os = "redox"))]
use std::os::unix::prelude::OsStrExt;
#[cfg(target_os = "wasi")]
Expand All @@ -2807,20 +2886,23 @@ fn path_to_file_url_segments(
Ok((host_end, HostInternal::None))
}

#[cfg(windows)]
#[cfg(all(feature = "std", windows))]
fn path_to_file_url_segments(
path: &Path,
serialization: &mut String,
) -> Result<(u32, HostInternal), ()> {
path_to_file_url_segments_windows(path, serialization)
}

#[cfg(feature = "std")]
// Build this unconditionally to alleviate https://github.com/servo/rust-url/issues/102
#[cfg_attr(not(windows), allow(dead_code))]
fn path_to_file_url_segments_windows(
path: &Path,
serialization: &mut String,
) -> Result<(u32, HostInternal), ()> {
use crate::parser::PATH_SEGMENT;
use percent_encoding::percent_encode;
use std::path::{Component, Prefix};
if !path.is_absolute() {
return Err(());
Expand Down Expand Up @@ -2879,11 +2961,12 @@ fn path_to_file_url_segments_windows(
Ok((host_end, host_internal))
}

#[cfg(any(unix, target_os = "redox", target_os = "wasi"))]
#[cfg(all(feature = "std", any(unix, target_os = "redox", target_os = "wasi")))]
fn file_url_segments_to_pathbuf(
host: Option<&str>,
segments: str::Split<'_, char>,
) -> Result<PathBuf, ()> {
use percent_encoding::percent_decode;
use std::ffi::OsStr;
#[cfg(any(unix, target_os = "redox"))]
use std::os::unix::prelude::OsStrExt;
Expand Down Expand Up @@ -2924,20 +3007,22 @@ fn file_url_segments_to_pathbuf(
Ok(path)
}

#[cfg(windows)]
#[cfg(all(feature = "std", windows))]
fn file_url_segments_to_pathbuf(
host: Option<&str>,
segments: str::Split<char>,
) -> Result<PathBuf, ()> {
file_url_segments_to_pathbuf_windows(host, segments)
}

#[cfg(feature = "std")]
// Build this unconditionally to alleviate https://github.com/servo/rust-url/issues/102
#[cfg_attr(not(windows), allow(dead_code))]
fn file_url_segments_to_pathbuf_windows(
host: Option<&str>,
mut segments: str::Split<'_, char>,
) -> Result<PathBuf, ()> {
use percent_encoding::percent_decode;
let mut string = if let Some(host) = host {
r"\\".to_owned() + host
} else {
Expand Down
3 changes: 2 additions & 1 deletion url/src/origin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
use crate::host::Host;
use crate::parser::default_port;
use crate::Url;
use std::sync::atomic::{AtomicUsize, Ordering};
use alloc::{borrow::ToOwned, string::String};
use core::sync::atomic::{AtomicUsize, Ordering};

pub fn url_origin(url: &Url) -> Origin {
let scheme = url.scheme();
Expand Down
13 changes: 7 additions & 6 deletions url/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::error::Error;
use std::fmt::{self, Formatter, Write};
use std::str;
use alloc::string::{String, ToString};
use core::fmt::{self, Formatter, Write};
use core::str;

use crate::host::{Host, HostInternal};
use crate::Url;
Expand Down Expand Up @@ -72,7 +72,8 @@ macro_rules! simple_enum_error {
}
}

impl Error for ParseError {}
#[cfg(feature = "std")]
impl std::error::Error for ParseError {}

simple_enum_error! {
EmptyHost => "empty host",
Expand Down Expand Up @@ -1107,7 +1108,7 @@ impl<'a> Parser<'a> {
while let (Some(c), remaining) = input.split_first() {
if let Some(digit) = c.to_digit(10) {
port = port * 10 + digit;
if port > ::std::u16::MAX as u32 {
if port > core::u16::MAX as u32 {
return Err(ParseError::InvalidPort);
}
has_any_digit = true;
Expand Down Expand Up @@ -1576,7 +1577,7 @@ pub fn ascii_alpha(ch: char) -> bool {

#[inline]
pub fn to_u32(i: usize) -> ParseResult<u32> {
if i <= ::std::u32::MAX as usize {
if i <= core::u32::MAX as usize {
Ok(i as u32)
} else {
Err(ParseError::Overflow)
Expand Down