Skip to content

Add SingleLineString to reject line breaks in RFD attribute values #615

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.

Sign up for GitHub

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

Merged
notpeter merged 2 commits into from
Nov 12, 2026
Merged

Add SingleLineString to reject line breaks in RFD attribute values #615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c05e8b0
Reject line breaks in RFD attribute values with SingleLineString
notpeter Nov 12, 2026
b7fb196
One less test
notpeter Nov 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ partial-struct = { git = "https://github.com/oxidecomputer/partial-struct" }
progenitor = { version = "0.15" }
progenitor-client = { version = "0.15" }
regex = "1.13.1"
regress = "0.12"
reqwest = { version = "0.13", default-features = false, features = ["json", "stream", "rustls"] }
reqwest-middleware = "0.5"
reqwest-retry = "0.9"
Expand Down
3 changes: 2 additions & 1 deletion rfd-api-spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4441,7 +4441,8 @@
},
"value": {
"description": "Full value to set this attribute to in the existing RFD contents",
"type": "string"
"type": "string",
"pattern": "^[^\\r\\n]*$"
}
},
"required": [
Expand Down
10 changes: 5 additions & 5 deletions rfd-api/src/endpoints/rfd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use dropshot::{
use newtype_uuid::TypedUuid;
use rfd_data::{
content::{RfdAsciidoc, RfdContent, RfdDocument, RfdMarkdown},
RfdState,
RfdState, SingleLineString,
};
use rfd_model::{
schema_ext::{ContentFormat, Visibility},
Expand Down Expand Up @@ -717,7 +717,7 @@ async fn set_rfd_content_op(
#[derive(Debug, Deserialize, Serialize, JsonSchema)]
pub struct RfdAttrValue {
/// Full value to set this attribute to in the existing RFD contents
value: String,
value: SingleLineString,
/// Optional Git commit message to send with this update (recommended)
message: Option<String>,
}
Expand Down Expand Up @@ -767,7 +767,7 @@ async fn set_rfd_attr_op(
tracing::info!(?err, "Invalid state was supplied");
HttpError::for_bad_request(None, "Invalid RFD state".to_string())
})?;
content.update_state(&state.to_string())
content.update_state(&state.into())
}
};

Expand Down Expand Up @@ -814,7 +814,7 @@ pub async fn discuss_rfd(
path.number,
RfdAttrName::State,
&RfdAttrValue {
value: RfdState::Discussion.to_string(),
value: RfdState::Discussion.into(),
message: Some("Move to discussion".to_string()),
},
)
Expand All @@ -841,7 +841,7 @@ pub async fn publish_rfd(
path.number,
RfdAttrName::State,
&RfdAttrValue {
value: RfdState::Published.to_string(),
value: RfdState::Published.into(),
message: Some("Publish".to_string()),
},
)
Expand Down
4 changes: 2 additions & 2 deletions rfd-cli/src/generated/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ impl<T: CliConfig> Cli<T> {
.arg(
::clap::Arg::new("value")
.long("value")
.value_parser(::clap::value_parser!(::std::string::String))
.value_parser(::clap::value_parser!(types::RfdAttrValueValue))
.required_unless_present("json-body")
.help("Full value to set this attribute to in the existing RFD contents"),
)
Expand Down Expand Up @@ -3437,7 +3437,7 @@ impl<T: CliConfig> Cli<T> {
request = request.number(value.clone());
}

if let Some(value) = matches.get_one::<::std::string::String>("value") {
if let Some(value) = matches.get_one::<types::RfdAttrValueValue>("value") {
request = request.body_map(|body| body.value(value.clone()))
}

Expand Down
3 changes: 3 additions & 0 deletions rfd-data/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ schemars = { workspace = true }
serde = { workspace = true }
thiserror = { workspace = true }
tracing = { workspace = true }

[dev-dependencies]
serde_json = { workspace = true }
21 changes: 13 additions & 8 deletions rfd-data/src/content/asciidoc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use regex::{regex, Regex};
use std::borrow::Cow;

use super::RfdDocument;
use crate::SingleLineString;

/// The text data of an Asciidoc RFD
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -189,7 +190,7 @@ impl<'a> RfdDocument for RfdAsciidoc<'a> {
Self::attr("state", &self.resolved)
}

fn update_state(&mut self, value: &str) -> &mut Self {
fn update_state(&mut self, value: &SingleLineString) -> &mut Self {
self.set_raw(&Self::set_attr(&self.content, "state", value.trim()))
}

Expand All @@ -202,7 +203,7 @@ impl<'a> RfdDocument for RfdAsciidoc<'a> {
}
}

fn update_discussion(&mut self, value: &str) -> &mut Self {
fn update_discussion(&mut self, value: &SingleLineString) -> &mut Self {
self.set_raw(&Self::set_attr(&self.content, "discussion", value.trim()))
}

Comment thread
notpeter marked this conversation as resolved.
Resolved
Expand All @@ -214,7 +215,7 @@ impl<'a> RfdDocument for RfdAsciidoc<'a> {
Self::attr("labels", &self.resolved)
}

fn update_labels(&mut self, value: &str) -> &mut Self {
fn update_labels(&mut self, value: &SingleLineString) -> &mut Self {
self.set_raw(&Self::set_attr(&self.content, "labels", value.trim()))
}

Expand Down Expand Up @@ -429,6 +430,10 @@ impl RfdAuthor {
mod tests {
use super::*;

fn line(value: &str) -> crate::SingleLineString {
value.to_string().try_into().unwrap()
}

// RFD Author tests

#[test]
Expand Down Expand Up @@ -709,7 +714,7 @@ sdf
:discussion: nope"#;

let mut rfd = RfdAsciidoc::new(content);
rfd.update_discussion(link);
rfd.update_discussion(&line(link));
let expected = r#"sdfsdf
= sdfgsd
discussion: fgsdfg
Expand All @@ -733,7 +738,7 @@ sdf
:discussion: nope"#;

let mut rfd = RfdAsciidoc::new(content);
rfd.update_discussion(link);
rfd.update_discussion(&line(link));
let expected = r#"sdfsdf
= sdfgsd
discussion: fgsdfg
Expand All @@ -757,7 +762,7 @@ dsfsdf
sdf
:state: nope"#;
let mut rfd = RfdAsciidoc::new(content);
rfd.update_state(state);
rfd.update_state(&line(state));
let expected = r#"sdfsdf
= sdfgsd
state: fgsdfg
Expand All @@ -779,7 +784,7 @@ dsfsdf
sdf
:state: nope"#;
let mut rfd = RfdAsciidoc::new(content);
rfd.update_state(state);
rfd.update_state(&line(state));
let expected = r#"sdfsdf
= sdfgsd
state: fgsdfg
Expand Down Expand Up @@ -921,7 +926,7 @@ in velit.
#[test]
fn test_update_asciidoc_labels() {
let mut rfd = RfdAsciidoc::new(Cow::Borrowed(test_rfd_content()));
rfd.update_labels("newlabel1, newlabel2");
rfd.update_labels(&line("newlabel1, newlabel2"));
let labels = rfd.get_labels().unwrap();
let expected = "newlabel1, newlabel2".to_string();
assert_eq!(expected, labels);
Expand Down
15 changes: 10 additions & 5 deletions rfd-data/src/content/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use regex::{regex, Regex};
use std::borrow::Cow;

use super::RfdDocument;
use crate::SingleLineString;

#[derive(Debug, Clone)]
pub struct RfdMarkdown<'a> {
Expand Down Expand Up @@ -95,7 +96,7 @@ impl<'a> RfdDocument for RfdMarkdown<'a> {
self.attr("state")
}

fn update_state(&mut self, value: &str) -> &mut Self {
fn update_state(&mut self, value: &SingleLineString) -> &mut Self {
self.set_attr("state", value);
self
}
Expand All @@ -104,7 +105,7 @@ impl<'a> RfdDocument for RfdMarkdown<'a> {
self.attr("discussion")
}

fn update_discussion(&mut self, value: &str) -> &mut Self {
fn update_discussion(&mut self, value: &SingleLineString) -> &mut Self {
self.set_attr("discussion", value);
self
}
Expand All @@ -117,7 +118,7 @@ impl<'a> RfdDocument for RfdMarkdown<'a> {
self.attr("labels")
}

fn update_labels(&mut self, value: &str) -> &mut Self {
fn update_labels(&mut self, value: &SingleLineString) -> &mut Self {
self.set_attr("labels", value);
self
}
Expand Down Expand Up @@ -160,6 +161,10 @@ impl<'a> RfdDocument for RfdMarkdown<'a> {
mod tests {
use crate::content::{markdown::RfdMarkdown, RfdDocument};

fn line(value: &str) -> crate::SingleLineString {
value.to_string().try_into().unwrap()
}

// Read authors tests

#[test]
Expand Down Expand Up @@ -247,7 +252,7 @@ dsfsdf
sdf
authors: nope"#;
let mut rfd = RfdMarkdown::new(content);
rfd.update_discussion(link);
rfd.update_discussion(&line(link));

let expected = r#"sdfsdf
sdfsdf
Expand All @@ -271,7 +276,7 @@ dsfsdf
sdf
authors: nope"#;
let mut rfd = RfdMarkdown::new(content);
rfd.update_state(state);
rfd.update_state(&line(state));

let expected = r#"sdfsdf
sdfsdf
Expand Down
14 changes: 8 additions & 6 deletions rfd-data/src/content/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ pub use template::{RenderableRfdTemplate, RfdTemplate, TemplateError};

use rfd_model::{schema_ext::ContentFormat, RfdRevision};

use crate::SingleLineString;

pub trait RfdDocument {
/// Extract the title from the internal content
fn get_title(&self) -> Option<&str>;
Expand All @@ -20,13 +22,13 @@ pub trait RfdDocument {
fn get_state(&self) -> Option<&str>;

// Update the state value stored within the document or add it if it does not exist
fn update_state(&mut self, value: &str) -> &mut Self;
fn update_state(&mut self, value: &SingleLineString) -> &mut Self;

/// Get the discussion link stored within the document
fn get_discussion(&self) -> Option<&str>;

// Update the discussion link stored within the document or add it if it does not exist
fn update_discussion(&mut self, value: &str) -> &mut Self;
fn update_discussion(&mut self, value: &SingleLineString) -> &mut Self;

/// Get the authors line stored within the document. The returned string may contain multiple
/// names
Expand All @@ -36,7 +38,7 @@ pub trait RfdDocument {
fn get_labels(&self) -> Option<&str>;

// Update the labels stored within the document or add them if they do not exist
fn update_labels(&mut self, value: &str) -> &mut Self;
fn update_labels(&mut self, value: &SingleLineString) -> &mut Self;

// Get a reference to the contents of the RFD header
fn header(&self) -> Option<&str>;
Expand Down Expand Up @@ -84,7 +86,7 @@ impl<'a> RfdDocument for RfdContent<'a> {
}
}

fn update_state(&mut self, value: &str) -> &mut Self {
fn update_state(&mut self, value: &SingleLineString) -> &mut Self {
match self {
Self::Asciidoc(inner) => {
inner.update_state(value);
Expand All @@ -103,7 +105,7 @@ impl<'a> RfdDocument for RfdContent<'a> {
}
}

fn update_discussion(&mut self, value: &str) -> &mut Self {
fn update_discussion(&mut self, value: &SingleLineString) -> &mut Self {
match self {
Self::Asciidoc(inner) => {
inner.update_discussion(value);
Expand All @@ -129,7 +131,7 @@ impl<'a> RfdDocument for RfdContent<'a> {
}
}

fn update_labels(&mut self, value: &str) -> &mut Self {
fn update_labels(&mut self, value: &SingleLineString) -> &mut Self {
match self {
Self::Asciidoc(inner) => {
inner.update_labels(value);
Expand Down
Loading
Loading