Skip to content
This repository was archived by the owner on Nov 22, 2018. It is now read-only.

Add RangeHelper #185

Merged
merged 3 commits into from
Apr 7, 2017
Merged

Add RangeHelper #185

merged 3 commits into from
Apr 7, 2017

Conversation

jbagga
Copy link
Contributor

@jbagga jbagga commented Apr 7, 2017

@dnfclas
Copy link

dnfclas commented Apr 7, 2017

@jbagga,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

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

Since this is back in the StaticFiles repo, you can now update StaticFiles to use it in the same PR.

using Microsoft.Extensions.Primitives;
using Microsoft.Net.Http.Headers;

namespace Microsoft.AspNetCore.RangeHelper.Internal
Copy link
Member

Choose a reason for hiding this comment

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

Microsoft.AspNetCore.Internal

@jbagga
Copy link
Contributor Author

jbagga commented Apr 7, 2017

@Tratcher Updated

// The spec allows for multiple ranges but we choose not to support them because the client may request
// very strange ranges (e.g. each byte separately, overlapping ranges, etc.) that could negatively
// impact the server. Ignore the header and serve the response normally.
_logger.LogMultipleFileRanges(rawRangeHeader.ToString());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The shared source does not have a logger. Should I add it? @Tratcher

Copy link
Member

Choose a reason for hiding this comment

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

No, because then you'll have to add LogMultipleFileRanges (which you should now remove).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants