Skip to content

Commit 1758de3

Browse files
milesjmilesj
authored
fix: Audit 10/09 (#176)
1 parent 8aa524b commit 1758de3

File tree

8 files changed

+118
-63
lines changed

8 files changed

+118
-63
lines changed

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
#### 🐞 Fixes
6+
7+
- Fixed an issue where source URLs that contain a query string would not be parsed.
8+
39
## 0.18.13
410

511
#### 🐞 Fixes

‎crates/schematic/src/config/source.rs‎

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use super::cacher::BoxedCacher;
22
use super::error::ConfigError;
33
use crate::format::Format;
4+
use crate::helpers::*;
45
use serde::Deserialize;
56
use serde::{Serialize, de::DeserializeOwned};
67
use std::fs;
@@ -169,44 +170,3 @@ impl Source {
169170
}
170171
}
171172
}
172-
173-
/// Returns true if the value ends in a supported file extension.
174-
pub fn is_source_format(value: &str) -> bool {
175-
value.ends_with(".json")
176-
|| value.ends_with(".pkl")
177-
|| value.ends_with(".toml")
178-
|| value.ends_with(".yaml")
179-
|| value.ends_with(".yml")
180-
}
181-
182-
/// Returns true if the value looks like a file, by checking for `file://`,
183-
/// path separators, or supported file extensions.
184-
pub fn is_file_like(value: &str) -> bool {
185-
value.starts_with("file://")
186-
|| value.starts_with('/')
187-
|| value.starts_with('\\')
188-
|| value.starts_with('.')
189-
|| value.contains('/')
190-
|| value.contains('\\')
191-
|| value.contains('.')
192-
}
193-
194-
/// Returns true if the value looks like a URL, by checking for `http://`, `https://`, or `www`.
195-
pub fn is_url_like(value: &str) -> bool {
196-
value.starts_with("https://") || value.starts_with("http://") || value.starts_with("www")
197-
}
198-
199-
/// Returns true if the value is a secure URL, by checking for `https://`. This check can be
200-
/// bypassed for localhost URLs.
201-
pub fn is_secure_url(value: &str) -> bool {
202-
if value.contains("127.0.0.1") || value.contains("//localhost") {
203-
return true;
204-
}
205-
206-
value.starts_with("https://")
207-
}
208-
209-
/// Strip a leading BOM from the string.
210-
pub fn strip_bom(content: &str) -> &str {
211-
content.trim_start_matches("\u{feff}")
212-
}

‎crates/schematic/src/format.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::helpers::extract_ext;
12
use miette::Diagnostic;
23
use serde::{Deserialize, Serialize};
34
use thiserror::Error;
@@ -33,12 +34,13 @@ impl Format {
3334
/// checking for a supported file extension.
3435
pub fn detect(value: &str) -> Result<Format, UnsupportedFormatError> {
3536
let mut available: Vec<&str> = vec![];
37+
let ext = extract_ext(value).unwrap_or_default();
3638

3739
#[cfg(feature = "json")]
3840
{
3941
available.push("JSON");
4042

41-
if value.ends_with(".json") {
43+
if ext == ".json" {
4244
return Ok(Format::Json);
4345
}
4446
}
@@ -47,7 +49,7 @@ impl Format {
4749
{
4850
available.push("Pkl");
4951

50-
if value.ends_with(".pkl") {
52+
if ext == ".pkl" {
5153
return Ok(Format::Pkl);
5254
}
5355
}
@@ -56,7 +58,7 @@ impl Format {
5658
{
5759
available.push("RON");
5860

59-
if value.ends_with(".ron") {
61+
if ext == ".ron" {
6062
return Ok(Format::Ron);
6163
}
6264
}
@@ -65,7 +67,7 @@ impl Format {
6567
{
6668
available.push("TOML");
6769

68-
if value.ends_with(".toml") {
70+
if ext == ".toml" {
6971
return Ok(Format::Toml);
7072
}
7173
}
@@ -74,7 +76,7 @@ impl Format {
7476
{
7577
available.push("YAML");
7678

77-
if value.ends_with(".yaml") || value.ends_with(".yml") {
79+
if ext == ".yaml" || ext == ".yml" {
7880
return Ok(Format::Yaml);
7981
}
8082
}

‎crates/schematic/src/helpers.rs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/// Returns true if the value ends in a supported file extension.
2+
pub fn is_source_format(value: &str) -> bool {
3+
extract_ext(value).is_some_and(|ext| {
4+
ext == ".json" || ext == ".pkl" || ext == ".toml" || ext == ".yaml" || ext == ".yml"
5+
})
6+
}
7+
8+
/// Returns true if the value looks like a file, by checking for `file://`,
9+
/// path separators, or supported file extensions.
10+
pub fn is_file_like(value: &str) -> bool {
11+
value.starts_with("file://")
12+
|| value.starts_with('/')
13+
|| value.starts_with('\\')
14+
|| value.starts_with('.')
15+
|| value.contains('/')
16+
|| value.contains('\\')
17+
|| value.contains('.')
18+
}
19+
20+
/// Returns true if the value looks like a URL, by checking for `http://`, `https://`, or `www`.
21+
pub fn is_url_like(value: &str) -> bool {
22+
value.starts_with("https://") || value.starts_with("http://") || value.starts_with("www")
23+
}
24+
25+
/// Returns true if the value is a secure URL, by checking for `https://`. This check can be
26+
/// bypassed for localhost URLs.
27+
pub fn is_secure_url(value: &str) -> bool {
28+
if value.contains("127.0.0.1") || value.contains("//localhost") {
29+
return true;
30+
}
31+
32+
value.starts_with("https://")
33+
}
34+
35+
/// Strip a leading BOM from the string.
36+
pub fn strip_bom(content: &str) -> &str {
37+
content.trim_start_matches("\u{feff}")
38+
}
39+
40+
/// Extract a file extension from the provided file path or URL.
41+
pub fn extract_ext(value: &str) -> Option<&str> {
42+
// Remove any query string
43+
let value = if let Some(index) = value.rfind('?') {
44+
&value[0..index]
45+
} else {
46+
value
47+
};
48+
49+
// And only check the last segment
50+
let value = if let Some(index) = value.rfind('/') {
51+
&value[index + 1..]
52+
} else {
53+
value
54+
};
55+
56+
value.rfind('.').map(|index| &value[index..])
57+
}

‎crates/schematic/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#![allow(clippy::result_large_err)]
22

33
mod format;
4+
pub mod helpers;
45

56
#[cfg(feature = "config")]
67
mod config;

‎crates/schematic/src/validate/extends.rs‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
use crate::config::{
2-
ExtendsFrom, Path, PathSegment, ValidateError, ValidateResult, is_file_like, is_secure_url,
3-
is_source_format, is_url_like,
4-
};
1+
use crate::config::{ExtendsFrom, Path, PathSegment, ValidateError, ValidateResult};
2+
use crate::helpers::*;
53

64
/// Validate an `extend` value is either a file path or secure URL.
75
pub fn extends_string<D, C>(
@@ -19,18 +17,10 @@ pub fn extends_string<D, C>(
1917
));
2018
}
2119

22-
if !value.is_empty() {
23-
let value = if is_url && let Some(index) = value.rfind('?') {
24-
&value[0..index]
25-
} else {
26-
value
27-
};
28-
29-
if !is_source_format(value) {
30-
return Err(ValidateError::new(
31-
"invalid file format, try a supported extension",
32-
));
33-
}
20+
if !value.is_empty() && !is_source_format(value) {
21+
return Err(ValidateError::new(
22+
"invalid file format, try a supported extension",
23+
));
3424
}
3525

3626
if is_url && !is_secure_url(value) {

‎crates/schematic/src/validate/url.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::{ValidateError, ValidateResult, map_err};
2-
use crate::config::is_secure_url;
2+
use crate::helpers::is_secure_url;
33
pub use garde::rules::url::Url;
44

55
/// Validate a string matches a URL.

‎crates/schematic/tests/helpers_test.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
use schematic::helpers::extract_ext;
2+
3+
mod ext {
4+
use super::*;
5+
6+
#[test]
7+
fn works_on_files() {
8+
assert!(extract_ext("file").is_none());
9+
assert_eq!(extract_ext("file.json").unwrap(), ".json");
10+
assert_eq!(extract_ext("dir/file.yaml").unwrap(), ".yaml");
11+
assert_eq!(extract_ext("../file.toml").unwrap(), ".toml");
12+
assert_eq!(extract_ext("/root/file.other.json").unwrap(), ".json");
13+
}
14+
15+
#[test]
16+
fn works_on_urls() {
17+
assert!(extract_ext("https://domain.com/file").is_none());
18+
assert_eq!(
19+
extract_ext("https://domain.com/file.json").unwrap(),
20+
".json"
21+
);
22+
assert_eq!(
23+
extract_ext("http://domain.com/dir/file.yaml").unwrap(),
24+
".yaml"
25+
);
26+
assert_eq!(
27+
extract_ext("https://domain.com/file.toml?query").unwrap(),
28+
".toml"
29+
);
30+
assert_eq!(
31+
extract_ext("http://domain.com/root/file.other.json").unwrap(),
32+
".json"
33+
);
34+
assert_eq!(
35+
extract_ext("https://domain.com/other.segment/file.toml?query").unwrap(),
36+
".toml"
37+
);
38+
}
39+
}

0 commit comments

Comments
 (0)