Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043

Merged
buyaa-n merged 2 commits into
dotnet:mainfrom
buyaa-n:max-encode-length
Oct 8, 2024
Merged

Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom
buyaa-n:max-encode-length

Conversation

@buyaa-n

@buyaa-nbuyaa-n commented Sep 19, 2024

Copy link
Copy Markdown
Contributor

BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago

As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.

Fixes#35142

This was referenced Sep 20, 2024
Closed
Closed
@buyaa-nbuyaa-n mentioned this pull request Sep 20, 2024
Closed
@build-analysisbuild-analysisBot mentioned this pull request Sep 20, 2024
Open
stephentoub
stephentoub reviewed Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
public const int Quality_Default = 4;
public const int Quality_Max = 11;
public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes

@stephentoubstephentoubOct 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where does this number come from?

@buyaa-nbuyaa-nOct 7, 2024•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number

After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`

The comment is not that clear, but do not have a better version

@stephentoubstephentoubOct 10, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.

@buyaa-nbuyaa-nOct 10, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024
Closed
Closed
@buyaa-n

buyaa-n commented Oct 8, 2024

Copy link
Copy Markdown
ContributorAuthor

/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause

@buyaa-n
buyaa-n merged commit 848cabd into dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n deleted the max-encode-length branch October 8, 2024 16:54
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value

2 participants

@buyaa-n@stephentoub
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043
Merged
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Oct 8, 2024
Merged
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Conversation
@buyaa-n
@buyaa-nbuyaa-n
commented
Sep 19, 2024

Copy link


Copy Markdown

Contributor
BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago
As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes#35142

This was referenced Sep 20, 2024

Closed


Closed

@buyaa-nbuyaa-n
mentioned this pull request
Sep 20, 2024

Closed

@build-analysisbuild-analysisBot
mentioned this pull request
Sep 20, 2024

Open

stephentoub
stephentoub
reviewed
Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
 public const int Quality_Default = 4;
 public const int Quality_Max = 11;
 public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
 public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes
@stephentoubstephentoubOct 5, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Where does this number come from?
@buyaa-nbuyaa-nOct 7, 2024•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
@stephentoubstephentoubOct 10, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
@buyaa-nbuyaa-nOct 10, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024

Closed


Closed

@buyaa-n
buyaa-n
commented
Oct 8, 2024

Copy link


Copy Markdown

ContributorAuthor
/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause
@buyaa-n
buyaa-n
merged commit 848cabd
into
dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n
deleted the

max-encode-length

branch
October 8, 2024 16:54
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value


2 participants

@buyaa-n@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043
Merged
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Oct 8, 2024
Merged
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Conversation
@buyaa-n
@buyaa-nbuyaa-n
commented
Sep 19, 2024

Copy link


Copy Markdown

Contributor
BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago
As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes#35142

This was referenced Sep 20, 2024

Closed


Closed

@buyaa-nbuyaa-n
mentioned this pull request
Sep 20, 2024

Closed

@build-analysisbuild-analysisBot
mentioned this pull request
Sep 20, 2024

Open

stephentoub
stephentoub
reviewed
Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
 public const int Quality_Default = 4;
 public const int Quality_Max = 11;
 public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
 public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes
@stephentoubstephentoubOct 5, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Where does this number come from?
@buyaa-nbuyaa-nOct 7, 2024•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
@stephentoubstephentoubOct 10, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
@buyaa-nbuyaa-nOct 10, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024

Closed


Closed

@buyaa-n
buyaa-n
commented
Oct 8, 2024

Copy link


Copy Markdown

ContributorAuthor
/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause
@buyaa-n
buyaa-n
merged commit 848cabd
into
dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n
deleted the

max-encode-length

branch
October 8, 2024 16:54
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value


2 participants

@buyaa-n@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043
Merged
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Oct 8, 2024
Merged
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Conversation
@buyaa-n
@buyaa-nbuyaa-n
commented
Sep 19, 2024

Copy link


Copy Markdown

Contributor
BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago
As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes#35142

This was referenced Sep 20, 2024

Closed


Closed

@buyaa-nbuyaa-n
mentioned this pull request
Sep 20, 2024

Closed

@build-analysisbuild-analysisBot
mentioned this pull request
Sep 20, 2024

Open

stephentoub
stephentoub
reviewed
Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
 public const int Quality_Default = 4;
 public const int Quality_Max = 11;
 public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
 public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes
@stephentoubstephentoubOct 5, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Where does this number come from?
@buyaa-nbuyaa-nOct 7, 2024•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
@stephentoubstephentoubOct 10, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
@buyaa-nbuyaa-nOct 10, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024

Closed


Closed

@buyaa-n
buyaa-n
commented
Oct 8, 2024

Copy link


Copy Markdown

ContributorAuthor
/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause
@buyaa-n
buyaa-n
merged commit 848cabd
into
dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n
deleted the

max-encode-length

branch
October 8, 2024 16:54
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value


2 participants

@buyaa-n@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043
Merged
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Oct 8, 2024
Merged
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Conversation
@buyaa-n
@buyaa-nbuyaa-n
commented
Sep 19, 2024

Copy link


Copy Markdown

Contributor
BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago
As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes#35142

This was referenced Sep 20, 2024

Closed


Closed

@buyaa-nbuyaa-n
mentioned this pull request
Sep 20, 2024

Closed

@build-analysisbuild-analysisBot
mentioned this pull request
Sep 20, 2024

Open

stephentoub
stephentoub
reviewed
Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
 public const int Quality_Default = 4;
 public const int Quality_Max = 11;
 public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
 public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes
@stephentoubstephentoubOct 5, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Where does this number come from?
@buyaa-nbuyaa-nOct 7, 2024•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
@stephentoubstephentoubOct 10, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
@buyaa-nbuyaa-nOct 10, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024

Closed


Closed

@buyaa-n
buyaa-n
commented
Oct 8, 2024

Copy link


Copy Markdown

ContributorAuthor
/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause
@buyaa-n
buyaa-n
merged commit 848cabd
into
dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n
deleted the

max-encode-length

branch
October 8, 2024 16:54
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value


2 participants

@buyaa-n@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043
Merged
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Oct 8, 2024
Merged
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Conversation
@buyaa-n
@buyaa-nbuyaa-n
commented
Sep 19, 2024

Copy link


Copy Markdown

Contributor
BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago
As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes#35142

This was referenced Sep 20, 2024

Closed


Closed

@buyaa-nbuyaa-n
mentioned this pull request
Sep 20, 2024

Closed

@build-analysisbuild-analysisBot
mentioned this pull request
Sep 20, 2024

Open

stephentoub
stephentoub
reviewed
Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
 public const int Quality_Default = 4;
 public const int Quality_Max = 11;
 public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
 public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes
@stephentoubstephentoubOct 5, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Where does this number come from?
@buyaa-nbuyaa-nOct 7, 2024•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
@stephentoubstephentoubOct 10, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
@buyaa-nbuyaa-nOct 10, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024

Closed


Closed

@buyaa-n
buyaa-n
commented
Oct 8, 2024

Copy link


Copy Markdown

ContributorAuthor
/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause
@buyaa-n
buyaa-n
merged commit 848cabd
into
dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n
deleted the

max-encode-length

branch
October 8, 2024 16:54
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value


2 participants

@buyaa-n@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043
Merged
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Oct 8, 2024
Merged
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Conversation
@buyaa-n
@buyaa-nbuyaa-n
commented
Sep 19, 2024

Copy link


Copy Markdown

Contributor
BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago
As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes#35142

This was referenced Sep 20, 2024

Closed


Closed

@buyaa-nbuyaa-n
mentioned this pull request
Sep 20, 2024

Closed

@build-analysisbuild-analysisBot
mentioned this pull request
Sep 20, 2024

Open

stephentoub
stephentoub
reviewed
Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
 public const int Quality_Default = 4;
 public const int Quality_Max = 11;
 public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
 public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes
@stephentoubstephentoubOct 5, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Where does this number come from?
@buyaa-nbuyaa-nOct 7, 2024•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
@stephentoubstephentoubOct 10, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
@buyaa-nbuyaa-nOct 10, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024

Closed


Closed

@buyaa-n
buyaa-n
commented
Oct 8, 2024

Copy link


Copy Markdown

ContributorAuthor
/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause
@buyaa-n
buyaa-n
merged commit 848cabd
into
dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n
deleted the

max-encode-length

branch
October 8, 2024 16:54
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value


2 participants

@buyaa-n@stephentoub

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength - #108043
Merged
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Oct 8, 2024
Merged
Call native BrotliEncoderMaxCompressedSize method for BrotliEncoder.GetMaxCompressedLength#108043
buyaa-n merged 2 commits into
dotnet:mainfrom 
buyaa-n:max-encode-length
Conversation
@buyaa-n
@buyaa-nbuyaa-n
commented
Sep 19, 2024

Copy link


Copy Markdown

Contributor
BrotliEncoder.GetMaxCompressedLength returns invalid value for some input because managed implementation differs from native Brotli native implementation that updated many years ago
As discussed in the issue we should use the native implementation since it's the source of truth, and this size needs to remain in sync with what the encoder actually does.
Fixes#35142

This was referenced Sep 20, 2024

Closed


Closed

@buyaa-nbuyaa-n
mentioned this pull request
Sep 20, 2024

Closed

@build-analysisbuild-analysisBot
mentioned this pull request
Sep 20, 2024

Open

stephentoub
stephentoub
reviewed
Oct 5, 2024
View reviewed changes
Comment threadsrc/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliUtils.cs
 public const int Quality_Default = 4;
 public const int Quality_Max = 11;
 public const int MaxInputSize = int.MaxValue - 515; // 515 is the max compressed extra bytes
 public const int MaxInputSize = int.MaxValue - 524_166; // 524_166 is the max compressed extra bytes
@stephentoubstephentoubOct 5, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Where does this number come from?
@buyaa-nbuyaa-nOct 7, 2024•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Before int.MaxValue - 515 = 2_147_483_132 was producing max compressed length, i.e. GetMaxCompressedLength(2_147_483_132)) returned int.MaxValue. Now this produces negative number
After testing with various values now this value equals 524_166 and GetMaxCompressedLength(int.MaxValue - 524_166))producesint.MaxValue`
The comment is not that clear, but do not have a better version
@stephentoubstephentoubOct 10, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Rather than having this hardcoded constant, though, couldn't we just call the native method and then throw ArgumentOutOfRangeException if the result was > int.MaxValue? Part of the point of this change was delegating all behavior to the native function, but we're lifting some of the behavior back up into the C# by hardcoding this constant, which in theory could be invalidated the next time we take a Brotli update.
@buyaa-nbuyaa-nOct 10, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Right, good point

This was referenced Oct 7, 2024

Closed


Closed

@buyaa-n
buyaa-n
commented
Oct 8, 2024

Copy link


Copy Markdown

ContributorAuthor
/ba-g test failures in chrome-DebuggerTests are unrelated and has no log (dead-lettered), though there is many issues for same tests probably have same root cause
@buyaa-n
buyaa-n
merged commit 848cabd
into
dotnet:mainOct 8, 2024
@buyaa-n
buyaa-n
deleted the

max-encode-length

branch
October 8, 2024 16:54
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Nov 10, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@stephentoubstephentoubstephentoub approved these changes
@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

Assignees

@buyaa-nbuyaa-n

Labels

area-System.IO.Compression

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

BrotliEncoder.GetMaxCompressedLength returns invalid value


2 participants

@buyaa-n@stephentoub