Skip to content
This repository was archived by the owner on Apr 20, 2026. It is now read-only.

Validate check run output text and summary length#118

Merged
mkarimi23 merged 3 commits into
masterfrom
validate_checkrun_output_length
Dec 9, 2022
Merged

Validate check run output text and summary length#118
mkarimi23 merged 3 commits into
masterfrom
validate_checkrun_output_length

Conversation

@mkarimi23
Copy link
Copy Markdown
Contributor

Automatically validates the maximum length of check run output properties.

GitHub does not validate these properly on their side (at least in GHE 3.2)
and returns 422 HTTP responses instead. To avoid that, let's validate the data
in this client library.

The values are taken from the official GitHub documentation

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 9, 2022

Codecov Report

Merging #118 (8af1820) into master (5dd80ee) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #118      +/-   ##
============================================
+ Coverage     73.50%   73.59%   +0.08%     
- Complexity      250      253       +3     
============================================
  Files            39       40       +1     
  Lines           887      890       +3     
  Branches         39       40       +1     
============================================
+ Hits            652      655       +3     
  Misses          210      210              
  Partials         25       25              
Impacted Files Coverage Δ
...a/com/spotify/github/v3/checks/CheckRunOutput.java 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment on lines +102 to +105
Preconditions.checkState(summary().map(String::length).orElse(0) <= 64 * 1024,
"'summary' exceeded max length of 65535 characters");
Preconditions.checkState(text().map(String::length).orElse(0) <= 64 * 1024,
"'text' exceeded max length of 65535 characters");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Preconditions.checkState(summary().map(String::length).orElse(0) <= 64 * 1024,
"'summary' exceeded max length of 65535 characters");
Preconditions.checkState(text().map(String::length).orElse(0) <= 64 * 1024,
"'text' exceeded max length of 65535 characters");
Preconditions.checkState(summary().map(String::length).orElse(0) < 64 * 1024,
"'summary' exceeded max length of 65535 characters");
Preconditions.checkState(text().map(String::length).orElse(0) < 64 * 1024,
"'text' exceeded max length of 65535 characters");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

64*1024 = 65536
Maximum length allowed is 65535 characters

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Already resolved, thanks

Comment thread src/test/java/com/spotify/github/v3/checks/CheckRunOutputTest.java Outdated
@mkarimi23 mkarimi23 merged commit a47c212 into master Dec 9, 2022
@mkarimi23 mkarimi23 deleted the validate_checkrun_output_length branch December 9, 2022 14:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants