Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Request CachePolicy isn't applied in HTTP request header #60913

Merged
merged 7 commits into from
Nov 9, 2021
Merged

Request CachePolicy isn't applied in HTTP request header #60913

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
59b5a15
Request CachePolicy isn't applied in HTTP request header
pedrobsaila Oct 27, 2021
3d4bf86
handle other CachePolicy levels
pedrobsaila Oct 27, 2021
e272a65
manage max-stale cache policy case
pedrobsaila Oct 27, 2021
9ed19f3
handle DefaultCachePolicy
pedrobsaila Nov 2, 2021
ac36aec
CachePolicy is nullable by default
pedrobsaila Nov 3, 2021
32dffd6
explicitely set DefaultCachePolicy and DefaultCachePolicy tests execu…
pedrobsaila Nov 6, 2021
f499323
fix minor issues
pedrobsaila Nov 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.IO;
using System.Net.Cache;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Net.Security;
using System.Net.Sockets;
using System.Runtime.Serialization;
Expand Down Expand Up @@ -1137,6 +1138,8 @@ private async Task<WebResponse> SendRequest(bool async)
request.Headers.Host = Host;
}

AddCacheControlHeaders(request);

// Copy the HttpWebRequest request headers from the WebHeaderCollection into HttpRequestMessage.Headers and
// HttpRequestMessage.Content.Headers.
foreach (string headerName in _webHeaderCollection)
Expand Down Expand Up @@ -1202,6 +1205,77 @@ private async Task<WebResponse> SendRequest(bool async)
}
}

private void AddCacheControlHeaders(HttpRequestMessage request)
{
if (CachePolicy != null)
{
if (request.Headers.CacheControl == null)
{
request.Headers.CacheControl = new CacheControlHeaderValue();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

CacheControl will always be null here. I would:

var cacheControl = new CacheControlHeaderValue();

And accessing request.Headers.CacheControl over and over will do non-trivial work, so I'd use this local variable in the switch body and then after setting everything:

request.Headers.CacheControl = cacheControl;


if (CachePolicy is HttpRequestCachePolicy httpRequestCachePolicy)
{
switch (httpRequestCachePolicy.Level)
{
case HttpRequestCacheLevel.NoCacheNoStore:
request.Headers.CacheControl.NoCache = true;
request.Headers.CacheControl.NoStore = true;
request.Headers.Pragma.Add(new NameValueHeaderValue("no-cache"));
break;
case HttpRequestCacheLevel.Reload:
request.Headers.CacheControl.NoCache = true;
request.Headers.Pragma.Add(new NameValueHeaderValue("no-cache"));
break;
case HttpRequestCacheLevel.CacheOnly:
scalablecory marked this conversation as resolved.
Show resolved Hide resolved
case HttpRequestCacheLevel.CacheOrNextCacheOnly:
request.Headers.CacheControl.OnlyIfCached = true;
break;
case HttpRequestCacheLevel.Default:
if (httpRequestCachePolicy.MinFresh > TimeSpan.Zero)
{
request.Headers.CacheControl.MinFresh = httpRequestCachePolicy.MinFresh;
}

if (httpRequestCachePolicy.MaxAge != TimeSpan.MaxValue)
{
request.Headers.CacheControl.MaxAge = httpRequestCachePolicy.MaxAge;
}

if (httpRequestCachePolicy.MaxStale > TimeSpan.Zero)
{
request.Headers.CacheControl.MaxStale = true;
request.Headers.CacheControl.MaxStaleLimit = httpRequestCachePolicy.MaxStale;
}

break;
case HttpRequestCacheLevel.Refresh:
request.Headers.CacheControl.MaxAge = TimeSpan.Zero;
request.Headers.Pragma.Add(new NameValueHeaderValue("no-cache"));
break;
}
}
else
{
switch (CachePolicy.Level)
{
case RequestCacheLevel.NoCacheNoStore:
request.Headers.CacheControl.NoCache = true;
request.Headers.CacheControl.NoStore = true;
request.Headers.Pragma.Add(new NameValueHeaderValue("no-cache"));
break;
case RequestCacheLevel.Reload:
request.Headers.CacheControl.NoCache = true;
request.Headers.Pragma.Add(new NameValueHeaderValue("no-cache"));
break;
case RequestCacheLevel.CacheOnly:
request.Headers.CacheControl.OnlyIfCached = true;
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

CacheOnly is intended to throw if it's not in the local cache already. This is different from OnlyIfCached, which is a directive to the server/proxy.

.NET no longer has a local cache, so this can always throw a WebException. The text used in Framework is "The request was aborted: The request cache-only policy does not allow a network request and the response is not found in cache." and should be added as a resource string.

}
}
}
}

public override IAsyncResult BeginGetResponse(AsyncCallback? callback, object? state)
{
CheckAbort();
Expand Down
67 changes: 67 additions & 0 deletions src/libraries/System.Net.Requests/tests/HttpWebRequestTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1924,6 +1924,73 @@ public void Abort_CreateRequestThenAbort_Success(Uri remoteServer)
request.Abort();
}

[Theory]
[InlineData(HttpRequestCacheLevel.NoCacheNoStore, null, null, new string[] { "Pragma: no-cache", "Cache-Control: no-store, no-cache"})]
[InlineData(HttpRequestCacheLevel.Reload, null, null, new string[] { "Pragma: no-cache", "Cache-Control: no-cache" })]
[InlineData(HttpRequestCacheLevel.CacheOnly, null, null, new string[] { "Cache-Control: only-if-cached" })]
[InlineData(HttpRequestCacheLevel.CacheOrNextCacheOnly, null, null, new string[] { "Cache-Control: only-if-cached" })]
[InlineData(HttpRequestCacheLevel.Default, HttpCacheAgeControl.MinFresh, 10, new string[] { "Cache-Control: min-fresh=10" })]
[InlineData(HttpRequestCacheLevel.Default, HttpCacheAgeControl.MaxAge, 10, new string[] { "Cache-Control: max-age=10" })]
[InlineData(HttpRequestCacheLevel.Default, HttpCacheAgeControl.MaxStale, 10, new string[] { "Cache-Control: max-stale=10" })]
[InlineData(HttpRequestCacheLevel.Refresh, null, null, new string[] { "Pragma: no-cache", "Cache-Control: max-age=0" })]
public async Task SendGetRequest_WithHttpCachePolicy_AddCacheHeaders(
HttpRequestCacheLevel requestCacheLevel, HttpCacheAgeControl? ageControl, int? age, string[] expectedHeaders)
{
await LoopbackServer.CreateServerAsync(async (server, uri) =>
{
HttpWebRequest request = WebRequest.CreateHttp(uri);
request.CachePolicy = ageControl != null ?
new HttpRequestCachePolicy(ageControl.Value, TimeSpan.FromSeconds((double)age))
: new HttpRequestCachePolicy(requestCacheLevel);
Task<WebResponse> getResponse = GetResponseAsync(request);

await server.AcceptConnectionAsync(async connection =>
{
List<string> headers = await connection.ReadRequestHeaderAndSendResponseAsync();

foreach (string header in expectedHeaders)
{
Assert.Contains(header, headers);
}
});

using (var response = (HttpWebResponse)await getResponse)
{
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
}
});
}

[Theory]
[InlineData(RequestCacheLevel.NoCacheNoStore, new string[] { "Pragma: no-cache", "Cache-Control: no-store, no-cache" })]
[InlineData(RequestCacheLevel.Reload, new string[] { "Pragma: no-cache", "Cache-Control: no-cache" })]
[InlineData(RequestCacheLevel.CacheOnly, new string[] { "Cache-Control: only-if-cached" })]
public async Task SendGetRequest_WithCachePolicy_AddCacheHeaders(
RequestCacheLevel requestCacheLevel, string[] expectedHeaders)
{
await LoopbackServer.CreateServerAsync(async (server, uri) =>
{
HttpWebRequest request = WebRequest.CreateHttp(uri);
request.CachePolicy = new RequestCachePolicy(requestCacheLevel);
Task<WebResponse> getResponse = GetResponseAsync(request);

await server.AcceptConnectionAsync(async connection =>
{
List<string> headers = await connection.ReadRequestHeaderAndSendResponseAsync();

foreach (string header in expectedHeaders)
{
Assert.Contains(header, headers);
}
});

using (var response = (HttpWebResponse)await getResponse)
{
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
}
});
}

private void RequestStreamCallback(IAsyncResult asynchronousResult)
{
RequestState state = (RequestState)asynchronousResult.AsyncState;
Expand Down