Skip to content

Commit

Permalink
Sanitize stray newlines at point of origin
Browse files Browse the repository at this point in the history
Mentioned in the comments: A user can add a stray newline to an input and mess up the formatting. The original implementation guarded against this at the point of origination. The basic concept is that user input isn't to be trusted, and at the point that they've given it to us, we can sanitize it for consistency https://github.com/heroku/buildpacks-ruby/blob/dda4ede413fc3fe4d6d2f2f63f039c7c1e5cc5fd/commons/src/output/build_log.rs#L224. With that concept in mind, the BuildOutput needs to own the consistency of the data it passes to other modules and functions.

The build output concept largely takes ownership over newlines between outputs so I feel comfortable trimming these. I picked shared entry points to trim before any writeln_now calls. 

I extended the test to check for trailing newlines in additional locations.
  • Loading branch information
schneems committed Feb 13, 2024
1 parent 5e20c41 commit 3ad2008
Showing 1 changed file with 11 additions and 5 deletions.
16 changes: 11 additions & 5 deletions libherokubuildpack/src/buildpack_output/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ where

fn write_paragraph(&mut self, color: &ANSI, s: impl AsRef<str>) {
let io = self.state.write_mut();
let contents = s.as_ref().trim();

if !io.was_paragraph {
writeln_now(io, "");
Expand All @@ -279,7 +280,7 @@ where
io,
ansi_escape::wrap_ansi_escape_each_line(
color,
prefix_lines(s.as_ref(), |_, line| {
prefix_lines(contents, |_, line| {
// Avoid adding trailing whitespace to the line, if there was none already.
// The `\n` case is required since `prefix_lines` uses `str::split_inclusive`,
// which preserves any trailing newline characters if present.
Expand Down Expand Up @@ -330,7 +331,7 @@ where
&mut self.state.write,
ansi_escape::wrap_ansi_escape_each_line(
&ANSI::BoldPurple,
format!("\n# {}\n", buildpack_name.as_ref()),
format!("\n# {}\n", buildpack_name.as_ref().trim()),
),
);

Expand All @@ -357,7 +358,7 @@ where
const PREFIX_REST: &'static str = " ";

fn style(s: impl AsRef<str>) -> String {
prefix_first_rest_lines(Self::PREFIX_FIRST, Self::PREFIX_REST, s.as_ref())
prefix_first_rest_lines(Self::PREFIX_FIRST, Self::PREFIX_REST, s.as_ref().trim())
}

/// Begin a new section of the buildpack output.
Expand Down Expand Up @@ -406,7 +407,7 @@ where
const CMD_INDENT: &'static str = " ";

fn style(s: impl AsRef<str>) -> String {
prefix_first_rest_lines(Self::PREFIX_FIRST, Self::PREFIX_REST, s.as_ref())
prefix_first_rest_lines(Self::PREFIX_FIRST, Self::PREFIX_REST, s.as_ref().trim())
}

/// Emit a step in the buildpack output within a section.
Expand Down Expand Up @@ -538,8 +539,11 @@ mod test {
#[test]
fn write_paragraph_empty_lines() {
let io = BuildpackOutput::new(Vec::new())
.start("Example Buildpack")
.start("Example Buildpack\n\n")
.warning("hello\n\n\t\t\nworld")
.section("Version\n\n")
.step("Installing\n\n")
.finish()
.finish();

let tab_char = '\t';
Expand All @@ -552,6 +556,8 @@ mod test {
! {tab_char}{tab_char}
! world
- Version
- Installing
- Done (finished in < 0.1s)
"};

Expand Down

0 comments on commit 3ad2008

Please sign in to comment.