[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425

Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged

[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom
backport/pr-81722-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 14, 2023•
edited by karelz
Loading

Copy link
Copy Markdown
Contributor

Backport of #81722 to release/7.0
Fixes#81506

/cc @antonfirsov@slovely

Customer Impact

We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.
In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.

While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.

Testing

Targeted test case added in the PR.

Risk

Low. This is a fairly trivial change.

@ghost

ghost commented Mar 14, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #81722 to release/7.0

/cc @antonfirsov@slovely

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-System.Net.Http

Milestone:-

@antonfirsov
antonfirsov requested a review from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot mentioned this pull request Mar 15, 2023
Closed
@karelzkarelz added this to the 7.0.x milestone Mar 22, 2023
@karelzkarelz added the Servicing-consider Issue for next servicing release review label Mar 22, 2023
@karelz

karelz commented Mar 22, 2023•
edited
Loading

Copy link
Copy Markdown
Member

This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.

@rbhandarbhanda modified the milestones: 7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 23, 2023
@carlossanlop

carlossanlop commented Mar 28, 2023

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot mentioned this pull request Mar 29, 2023
Closed
@antonfirsov

antonfirsov commented Mar 29, 2023

Copy link
Copy Markdown
Contributor

/azp run runtime

@azure-pipelines

azure-pipelinesBot commented Mar 29, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@antonfirsov

antonfirsov commented Mar 30, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Test failures are unrelated.

All requirements listed in #83425 (comment) are met, I think we can merge this.

@antonfirsov

antonfirsov commented Mar 30, 2023

Copy link
Copy Markdown
Contributor

Repo maintainers will now be allowed to merge their own servicing PR

@carlossanlop I don't have the rights to merge this.

@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

carlossanlop commented Mar 30, 2023

Copy link
Copy Markdown
Contributor

I don't have the rights to merge this.

Fixed. Is the squash and merge button enabled for you now, @antonfirsov?

We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs

@antonfirsov

antonfirsov commented Mar 30, 2023

Copy link
Copy Markdown
Contributor

@carlossanlop no, this is what I see:

image

@carlossanlop

carlossanlop commented Mar 30, 2023

Copy link
Copy Markdown
Contributor

Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?

@antonfirsov
antonfirsov merged commit 88b65f9 into release/7.0-stagingMar 30, 2023
@antonfirsov

antonfirsov commented Mar 30, 2023

Copy link
Copy Markdown
Contributor

@carlossanlop was good now, merged, thanks for the help!

@carlossanlop
carlossanlop deleted the backport/pr-81722-to-release/7.0 branch March 30, 2023 21:40
@antonfirsovantonfirsov mentioned this pull request Mar 30, 2023
Closed
@ghostghost locked as resolved and limited conversation to collaborators Apr 30, 2023
@rbhandarbhanda modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects

None yet

Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely
, '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
[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425
Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged
[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Conversation
@github-actions
@github-actionsgithub-actionsBot
commented
Mar 14, 2023•

edited by karelz

Loading

Copy link


Copy Markdown

Contributor
Backport of #81722 to release/7.0
Fixes#81506
/cc @antonfirsov@slovely
Customer Impact
We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.

In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.
While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.
Testing
Targeted test case added in the PR.
Risk
Low. This is a fairly trivial change.
@ghost
ghost
commented
Mar 14, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/ncl

See info in area-owners.md if you want to be subscribed.
Issue Details
Backport of #81722 to release/7.0
/cc @antonfirsov@slovely
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
Author:github-actions[bot]
Assignees:-
Labels:
area-System.Net.Http
Milestone:-
@antonfirsov
antonfirsov
requested a review
from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 15, 2023

Closed

@karelzkarelz
added this to the 7.0.x milestone
Mar 22, 2023
@karelzkarelz
added
the 
Servicing-consider
Issue for next servicing release review
label
Mar 22, 2023
@karelz
karelz
commented
Mar 22, 2023•

edited

Loading

Copy link


Copy Markdown

Member
This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.
@rbhandarbhanda
modified the milestones:
7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda
added

Servicing-approved
Approved for servicing release
and removed

Servicing-consider
Issue for next servicing release review
labels
Mar 23, 2023
@carlossanlop
carlossanlop
commented
Mar 28, 2023

Copy link


Copy Markdown

Contributor
I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.
Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.
The new process is described here: runtime/docs/project/library-servicing.md.
The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.
Let me know if you have any questions.
@carlossanlop
carlossanlop
changed the base branch from
release/7.0
to
release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 29, 2023

Closed

@antonfirsov
antonfirsov
commented
Mar 29, 2023

Copy link


Copy Markdown

Contributor
/azp run runtime
@azure-pipelines
azure-pipelinesBot
commented
Mar 29, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@antonfirsov
antonfirsov
commented
Mar 30, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Test failures are unrelated.
All requirements listed in #83425 (comment) are met, I think we can merge this.
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Repo maintainers will now be allowed to merge their own servicing PR
@carlossanlop I don't have the rights to merge this.
@carlossanlopcarlossanlop
added

Servicing-approved
Approved for servicing release
and removed

Servicing-approved
Approved for servicing release
labels
Mar 30, 2023
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
I don't have the rights to merge this.
Fixed. Is the squash and merge button enabled for you now, @antonfirsov?
We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop no, this is what I see:
image
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?
@antonfirsov
antonfirsov
merged commit 88b65f9
into
release/7.0-stagingMar 30, 2023
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop was good now, merged, thanks for the help!
@carlossanlop
carlossanlop
deleted the

backport/pr-81722-to-release/7.0

branch
March 30, 2023 21:40
@antonfirsovantonfirsov
mentioned this pull request
Mar 30, 2023

Closed

@ghostghost
locked as resolved and limited conversation to collaborators
Apr 30, 2023
@rbhandarbhanda
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
@leecowleecow
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects


None yet


Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely

, '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
[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425
Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged
[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Conversation
@github-actions
@github-actionsgithub-actionsBot
commented
Mar 14, 2023•

edited by karelz

Loading

Copy link


Copy Markdown

Contributor
Backport of #81722 to release/7.0
Fixes#81506
/cc @antonfirsov@slovely
Customer Impact
We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.

In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.
While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.
Testing
Targeted test case added in the PR.
Risk
Low. This is a fairly trivial change.
@ghost
ghost
commented
Mar 14, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/ncl

See info in area-owners.md if you want to be subscribed.
Issue Details
Backport of #81722 to release/7.0
/cc @antonfirsov@slovely
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
Author:github-actions[bot]
Assignees:-
Labels:
area-System.Net.Http
Milestone:-
@antonfirsov
antonfirsov
requested a review
from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 15, 2023

Closed

@karelzkarelz
added this to the 7.0.x milestone
Mar 22, 2023
@karelzkarelz
added
the 
Servicing-consider
Issue for next servicing release review
label
Mar 22, 2023
@karelz
karelz
commented
Mar 22, 2023•

edited

Loading

Copy link


Copy Markdown

Member
This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.
@rbhandarbhanda
modified the milestones:
7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda
added

Servicing-approved
Approved for servicing release
and removed

Servicing-consider
Issue for next servicing release review
labels
Mar 23, 2023
@carlossanlop
carlossanlop
commented
Mar 28, 2023

Copy link


Copy Markdown

Contributor
I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.
Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.
The new process is described here: runtime/docs/project/library-servicing.md.
The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.
Let me know if you have any questions.
@carlossanlop
carlossanlop
changed the base branch from
release/7.0
to
release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 29, 2023

Closed

@antonfirsov
antonfirsov
commented
Mar 29, 2023

Copy link


Copy Markdown

Contributor
/azp run runtime
@azure-pipelines
azure-pipelinesBot
commented
Mar 29, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@antonfirsov
antonfirsov
commented
Mar 30, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Test failures are unrelated.
All requirements listed in #83425 (comment) are met, I think we can merge this.
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Repo maintainers will now be allowed to merge their own servicing PR
@carlossanlop I don't have the rights to merge this.
@carlossanlopcarlossanlop
added

Servicing-approved
Approved for servicing release
and removed

Servicing-approved
Approved for servicing release
labels
Mar 30, 2023
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
I don't have the rights to merge this.
Fixed. Is the squash and merge button enabled for you now, @antonfirsov?
We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop no, this is what I see:
image
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?
@antonfirsov
antonfirsov
merged commit 88b65f9
into
release/7.0-stagingMar 30, 2023
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop was good now, merged, thanks for the help!
@carlossanlop
carlossanlop
deleted the

backport/pr-81722-to-release/7.0

branch
March 30, 2023 21:40
@antonfirsovantonfirsov
mentioned this pull request
Mar 30, 2023

Closed

@ghostghost
locked as resolved and limited conversation to collaborators
Apr 30, 2023
@rbhandarbhanda
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
@leecowleecow
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects


None yet


Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely

, '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
[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425
Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged
[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Conversation
@github-actions
@github-actionsgithub-actionsBot
commented
Mar 14, 2023•

edited by karelz

Loading

Copy link


Copy Markdown

Contributor
Backport of #81722 to release/7.0
Fixes#81506
/cc @antonfirsov@slovely
Customer Impact
We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.

In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.
While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.
Testing
Targeted test case added in the PR.
Risk
Low. This is a fairly trivial change.
@ghost
ghost
commented
Mar 14, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/ncl

See info in area-owners.md if you want to be subscribed.
Issue Details
Backport of #81722 to release/7.0
/cc @antonfirsov@slovely
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
Author:github-actions[bot]
Assignees:-
Labels:
area-System.Net.Http
Milestone:-
@antonfirsov
antonfirsov
requested a review
from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 15, 2023

Closed

@karelzkarelz
added this to the 7.0.x milestone
Mar 22, 2023
@karelzkarelz
added
the 
Servicing-consider
Issue for next servicing release review
label
Mar 22, 2023
@karelz
karelz
commented
Mar 22, 2023•

edited

Loading

Copy link


Copy Markdown

Member
This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.
@rbhandarbhanda
modified the milestones:
7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda
added

Servicing-approved
Approved for servicing release
and removed

Servicing-consider
Issue for next servicing release review
labels
Mar 23, 2023
@carlossanlop
carlossanlop
commented
Mar 28, 2023

Copy link


Copy Markdown

Contributor
I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.
Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.
The new process is described here: runtime/docs/project/library-servicing.md.
The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.
Let me know if you have any questions.
@carlossanlop
carlossanlop
changed the base branch from
release/7.0
to
release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 29, 2023

Closed

@antonfirsov
antonfirsov
commented
Mar 29, 2023

Copy link


Copy Markdown

Contributor
/azp run runtime
@azure-pipelines
azure-pipelinesBot
commented
Mar 29, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@antonfirsov
antonfirsov
commented
Mar 30, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Test failures are unrelated.
All requirements listed in #83425 (comment) are met, I think we can merge this.
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Repo maintainers will now be allowed to merge their own servicing PR
@carlossanlop I don't have the rights to merge this.
@carlossanlopcarlossanlop
added

Servicing-approved
Approved for servicing release
and removed

Servicing-approved
Approved for servicing release
labels
Mar 30, 2023
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
I don't have the rights to merge this.
Fixed. Is the squash and merge button enabled for you now, @antonfirsov?
We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop no, this is what I see:
image
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?
@antonfirsov
antonfirsov
merged commit 88b65f9
into
release/7.0-stagingMar 30, 2023
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop was good now, merged, thanks for the help!
@carlossanlop
carlossanlop
deleted the

backport/pr-81722-to-release/7.0

branch
March 30, 2023 21:40
@antonfirsovantonfirsov
mentioned this pull request
Mar 30, 2023

Closed

@ghostghost
locked as resolved and limited conversation to collaborators
Apr 30, 2023
@rbhandarbhanda
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
@leecowleecow
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects


None yet


Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely

, '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
[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425
Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged
[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Conversation
@github-actions
@github-actionsgithub-actionsBot
commented
Mar 14, 2023•

edited by karelz

Loading

Copy link


Copy Markdown

Contributor
Backport of #81722 to release/7.0
Fixes#81506
/cc @antonfirsov@slovely
Customer Impact
We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.

In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.
While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.
Testing
Targeted test case added in the PR.
Risk
Low. This is a fairly trivial change.
@ghost
ghost
commented
Mar 14, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/ncl

See info in area-owners.md if you want to be subscribed.
Issue Details
Backport of #81722 to release/7.0
/cc @antonfirsov@slovely
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
Author:github-actions[bot]
Assignees:-
Labels:
area-System.Net.Http
Milestone:-
@antonfirsov
antonfirsov
requested a review
from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 15, 2023

Closed

@karelzkarelz
added this to the 7.0.x milestone
Mar 22, 2023
@karelzkarelz
added
the 
Servicing-consider
Issue for next servicing release review
label
Mar 22, 2023
@karelz
karelz
commented
Mar 22, 2023•

edited

Loading

Copy link


Copy Markdown

Member
This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.
@rbhandarbhanda
modified the milestones:
7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda
added

Servicing-approved
Approved for servicing release
and removed

Servicing-consider
Issue for next servicing release review
labels
Mar 23, 2023
@carlossanlop
carlossanlop
commented
Mar 28, 2023

Copy link


Copy Markdown

Contributor
I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.
Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.
The new process is described here: runtime/docs/project/library-servicing.md.
The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.
Let me know if you have any questions.
@carlossanlop
carlossanlop
changed the base branch from
release/7.0
to
release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 29, 2023

Closed

@antonfirsov
antonfirsov
commented
Mar 29, 2023

Copy link


Copy Markdown

Contributor
/azp run runtime
@azure-pipelines
azure-pipelinesBot
commented
Mar 29, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@antonfirsov
antonfirsov
commented
Mar 30, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Test failures are unrelated.
All requirements listed in #83425 (comment) are met, I think we can merge this.
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Repo maintainers will now be allowed to merge their own servicing PR
@carlossanlop I don't have the rights to merge this.
@carlossanlopcarlossanlop
added

Servicing-approved
Approved for servicing release
and removed

Servicing-approved
Approved for servicing release
labels
Mar 30, 2023
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
I don't have the rights to merge this.
Fixed. Is the squash and merge button enabled for you now, @antonfirsov?
We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop no, this is what I see:
image
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?
@antonfirsov
antonfirsov
merged commit 88b65f9
into
release/7.0-stagingMar 30, 2023
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop was good now, merged, thanks for the help!
@carlossanlop
carlossanlop
deleted the

backport/pr-81722-to-release/7.0

branch
March 30, 2023 21:40
@antonfirsovantonfirsov
mentioned this pull request
Mar 30, 2023

Closed

@ghostghost
locked as resolved and limited conversation to collaborators
Apr 30, 2023
@rbhandarbhanda
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
@leecowleecow
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects


None yet


Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely

, '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
[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425
Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged
[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Conversation
@github-actions
@github-actionsgithub-actionsBot
commented
Mar 14, 2023•

edited by karelz

Loading

Copy link


Copy Markdown

Contributor
Backport of #81722 to release/7.0
Fixes#81506
/cc @antonfirsov@slovely
Customer Impact
We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.

In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.
While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.
Testing
Targeted test case added in the PR.
Risk
Low. This is a fairly trivial change.
@ghost
ghost
commented
Mar 14, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/ncl

See info in area-owners.md if you want to be subscribed.
Issue Details
Backport of #81722 to release/7.0
/cc @antonfirsov@slovely
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
Author:github-actions[bot]
Assignees:-
Labels:
area-System.Net.Http
Milestone:-
@antonfirsov
antonfirsov
requested a review
from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 15, 2023

Closed

@karelzkarelz
added this to the 7.0.x milestone
Mar 22, 2023
@karelzkarelz
added
the 
Servicing-consider
Issue for next servicing release review
label
Mar 22, 2023
@karelz
karelz
commented
Mar 22, 2023•

edited

Loading

Copy link


Copy Markdown

Member
This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.
@rbhandarbhanda
modified the milestones:
7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda
added

Servicing-approved
Approved for servicing release
and removed

Servicing-consider
Issue for next servicing release review
labels
Mar 23, 2023
@carlossanlop
carlossanlop
commented
Mar 28, 2023

Copy link


Copy Markdown

Contributor
I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.
Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.
The new process is described here: runtime/docs/project/library-servicing.md.
The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.
Let me know if you have any questions.
@carlossanlop
carlossanlop
changed the base branch from
release/7.0
to
release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 29, 2023

Closed

@antonfirsov
antonfirsov
commented
Mar 29, 2023

Copy link


Copy Markdown

Contributor
/azp run runtime
@azure-pipelines
azure-pipelinesBot
commented
Mar 29, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@antonfirsov
antonfirsov
commented
Mar 30, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Test failures are unrelated.
All requirements listed in #83425 (comment) are met, I think we can merge this.
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Repo maintainers will now be allowed to merge their own servicing PR
@carlossanlop I don't have the rights to merge this.
@carlossanlopcarlossanlop
added

Servicing-approved
Approved for servicing release
and removed

Servicing-approved
Approved for servicing release
labels
Mar 30, 2023
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
I don't have the rights to merge this.
Fixed. Is the squash and merge button enabled for you now, @antonfirsov?
We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop no, this is what I see:
image
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?
@antonfirsov
antonfirsov
merged commit 88b65f9
into
release/7.0-stagingMar 30, 2023
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop was good now, merged, thanks for the help!
@carlossanlop
carlossanlop
deleted the

backport/pr-81722-to-release/7.0

branch
March 30, 2023 21:40
@antonfirsovantonfirsov
mentioned this pull request
Mar 30, 2023

Closed

@ghostghost
locked as resolved and limited conversation to collaborators
Apr 30, 2023
@rbhandarbhanda
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
@leecowleecow
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects


None yet


Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely

, '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
[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425
Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged
[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Conversation
@github-actions
@github-actionsgithub-actionsBot
commented
Mar 14, 2023•

edited by karelz

Loading

Copy link


Copy Markdown

Contributor
Backport of #81722 to release/7.0
Fixes#81506
/cc @antonfirsov@slovely
Customer Impact
We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.

In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.
While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.
Testing
Targeted test case added in the PR.
Risk
Low. This is a fairly trivial change.
@ghost
ghost
commented
Mar 14, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/ncl

See info in area-owners.md if you want to be subscribed.
Issue Details
Backport of #81722 to release/7.0
/cc @antonfirsov@slovely
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
Author:github-actions[bot]
Assignees:-
Labels:
area-System.Net.Http
Milestone:-
@antonfirsov
antonfirsov
requested a review
from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 15, 2023

Closed

@karelzkarelz
added this to the 7.0.x milestone
Mar 22, 2023
@karelzkarelz
added
the 
Servicing-consider
Issue for next servicing release review
label
Mar 22, 2023
@karelz
karelz
commented
Mar 22, 2023•

edited

Loading

Copy link


Copy Markdown

Member
This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.
@rbhandarbhanda
modified the milestones:
7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda
added

Servicing-approved
Approved for servicing release
and removed

Servicing-consider
Issue for next servicing release review
labels
Mar 23, 2023
@carlossanlop
carlossanlop
commented
Mar 28, 2023

Copy link


Copy Markdown

Contributor
I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.
Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.
The new process is described here: runtime/docs/project/library-servicing.md.
The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.
Let me know if you have any questions.
@carlossanlop
carlossanlop
changed the base branch from
release/7.0
to
release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 29, 2023

Closed

@antonfirsov
antonfirsov
commented
Mar 29, 2023

Copy link


Copy Markdown

Contributor
/azp run runtime
@azure-pipelines
azure-pipelinesBot
commented
Mar 29, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@antonfirsov
antonfirsov
commented
Mar 30, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Test failures are unrelated.
All requirements listed in #83425 (comment) are met, I think we can merge this.
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Repo maintainers will now be allowed to merge their own servicing PR
@carlossanlop I don't have the rights to merge this.
@carlossanlopcarlossanlop
added

Servicing-approved
Approved for servicing release
and removed

Servicing-approved
Approved for servicing release
labels
Mar 30, 2023
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
I don't have the rights to merge this.
Fixed. Is the squash and merge button enabled for you now, @antonfirsov?
We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop no, this is what I see:
image
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?
@antonfirsov
antonfirsov
merged commit 88b65f9
into
release/7.0-stagingMar 30, 2023
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop was good now, merged, thanks for the help!
@carlossanlop
carlossanlop
deleted the

backport/pr-81722-to-release/7.0

branch
March 30, 2023 21:40
@antonfirsovantonfirsov
mentioned this pull request
Mar 30, 2023

Closed

@ghostghost
locked as resolved and limited conversation to collaborators
Apr 30, 2023
@rbhandarbhanda
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
@leecowleecow
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects


None yet


Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely

, '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
[release/7.0] Default media type used when 'null' is passed to StringContent #81506 - #83425
Merged
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Mar 30, 2023
Merged
[release/7.0] Default media type used when 'null' is passed to StringContent #81506#83425
antonfirsov merged 1 commit into
release/7.0-stagingfrom 
backport/pr-81722-to-release/7.0
Conversation
@github-actions
@github-actionsgithub-actionsBot
commented
Mar 14, 2023•

edited by karelz

Loading

Copy link


Copy Markdown

Contributor
Backport of #81722 to release/7.0
Fixes#81506
/cc @antonfirsov@slovely
Customer Impact
We introduced an unintentional breaking change/regression in one of StringContent's constructors in .NET 7.0: passing null to the arg string contentType leads to an exception.

In .NET 6.0 we would instead use default value text/plain - see .NET 6.0 code.
While it is easy to work around (check for null and pass 'text/plain' from user code), it has hit at least 3 customers (one of them was RestSharp - set of their customers hit it), so it is annoying, and it requires customers to change their code.
Testing
Targeted test case added in the PR.
Risk
Low. This is a fairly trivial change.
@ghost
ghost
commented
Mar 14, 2023

Copy link


Copy Markdown

Tagging subscribers to this area: @dotnet/ncl

See info in area-owners.md if you want to be subscribed.
Issue Details
Backport of #81722 to release/7.0
/cc @antonfirsov@slovely
Customer Impact
Testing
Risk
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
Author:github-actions[bot]
Assignees:-
Labels:
area-System.Net.Http
Milestone:-
@antonfirsov
antonfirsov
requested a review
from a teamMarch 14, 2023 22:26
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 15, 2023

Closed

@karelzkarelz
added this to the 7.0.x milestone
Mar 22, 2023
@karelzkarelz
added
the 
Servicing-consider
Issue for next servicing release review
label
Mar 22, 2023
@karelz
karelz
commented
Mar 22, 2023•

edited

Loading

Copy link


Copy Markdown

Member
This is annoying enough for customers (see updated Servicing template), that we should consider it for 7.0 servicing. Marking as such.
@rbhandarbhanda
modified the milestones:
7.0.x, 7.0.6Mar 23, 2023
@rbhandarbhanda
added

Servicing-approved
Approved for servicing release
and removed

Servicing-consider
Issue for next servicing release review
labels
Mar 23, 2023
@carlossanlop
carlossanlop
commented
Mar 28, 2023

Copy link


Copy Markdown

Contributor
I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.
Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:
  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.
The new process is described here: runtime/docs/project/library-servicing.md.
The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.
Let me know if you have any questions.
@carlossanlop
carlossanlop
changed the base branch from
release/7.0
to
release/7.0-stagingMarch 28, 2023 20:58
@build-analysisbuild-analysisBot
mentioned this pull request
Mar 29, 2023

Closed

@antonfirsov
antonfirsov
commented
Mar 29, 2023

Copy link


Copy Markdown

Contributor
/azp run runtime
@azure-pipelines
azure-pipelinesBot
commented
Mar 29, 2023

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@antonfirsov
antonfirsov
commented
Mar 30, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Test failures are unrelated.
All requirements listed in #83425 (comment) are met, I think we can merge this.
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Repo maintainers will now be allowed to merge their own servicing PR
@carlossanlop I don't have the rights to merge this.
@carlossanlopcarlossanlop
added

Servicing-approved
Approved for servicing release
and removed

Servicing-approved
Approved for servicing release
labels
Mar 30, 2023
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
I don't have the rights to merge this.
Fixed. Is the squash and merge button enabled for you now, @antonfirsov?
We had some weird issues with GitHub Actions yesterday and the new check-service-labels CI leg stopped working in servicing PRs. We made some changes and now the fix is to remove and reapply the Servicing-approved label to force the CI leg to retrigger and pass. cc @hoyosjs
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop no, this is what I see:
image
@carlossanlop
carlossanlop
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
Ah, we had an unnecessary restriction, I removed it. Can you please refresh? Is the "squash and merge" button enabled for you now, @antonfirsov?
@antonfirsov
antonfirsov
merged commit 88b65f9
into
release/7.0-stagingMar 30, 2023
@antonfirsov
antonfirsov
commented
Mar 30, 2023

Copy link


Copy Markdown

Contributor
@carlossanlop was good now, merged, thanks for the help!
@carlossanlop
carlossanlop
deleted the

backport/pr-81722-to-release/7.0

branch
March 30, 2023 21:40
@antonfirsovantonfirsov
mentioned this pull request
Mar 30, 2023

Closed

@ghostghost
locked as resolved and limited conversation to collaborators
Apr 30, 2023
@rbhandarbhanda
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
@leecowleecow
modified the milestones:
7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@karelzkarelzkarelz approved these changes
@MihaZupanMihaZupanMihaZupan approved these changes

Assignees

@antonfirsovantonfirsov

Labels

area-System.Net.Httpregression-from-last-releaseServicing-approvedApproved for servicing release

Projects


None yet


Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@karelz@carlossanlop@antonfirsov@MihaZupan@leecow@rbhanda@slovely