Skip to content

Commit cb2848c

Browse files
CarloTosoCarloToso
authored
Code cleanup for Get-Content to make -Head and -Tail disallow negative values (#19715)
1 parent bdf21cc commit cb2848c

2 files changed

Lines changed: 75 additions & 106 deletions

File tree

‎src/Microsoft.PowerShell.Commands.Management/commands/management/GetContentCommand.cs‎

Lines changed: 63 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -31,48 +31,20 @@ public class GetContentCommand : ContentCommandBase
3131
public long ReadCount { get; set; } = 1;
3232

3333
/// <summary>
34-
/// The number of content items to retrieve. By default this
35-
/// value is -1 which means read all the content.
34+
/// The number of content items to retrieve.
3635
/// </summary>
3736
[Parameter(ValueFromPipelineByPropertyName = true)]
37+
[ValidateRange(0, long.MaxValue)]
3838
[Alias("First", "Head")]
39-
public long TotalCount
40-
{
41-
get
42-
{
43-
return _totalCount;
44-
}
45-
46-
set
47-
{
48-
_totalCount = value;
49-
_totalCountSpecified = true;
50-
}
51-
}
52-
53-
private bool _totalCountSpecified = false;
39+
public long TotalCount { get; set; } = -1;
5440

5541
/// <summary>
5642
/// The number of content items to retrieve from the back of the file.
5743
/// </summary>
5844
[Parameter(ValueFromPipelineByPropertyName = true)]
45+
[ValidateRange(0, int.MaxValue)]
5946
[Alias("Last")]
60-
public int Tail
61-
{
62-
get
63-
{
64-
return _backCount;
65-
}
66-
67-
set
68-
{
69-
_backCount = value;
70-
_tailSpecified = true;
71-
}
72-
}
73-
74-
private int _backCount = -1;
75-
private bool _tailSpecified = false;
47+
public int Tail { get; set; } = -1;
7648

7749
/// <summary>
7850
/// A virtual method for retrieving the dynamic parameters for a cmdlet. Derived cmdlets
@@ -98,15 +70,6 @@ internal override object GetDynamicParameters(CmdletProviderContext context)
9870

9971
#endregion Parameters
10072

101-
#region parameter data
102-
103-
/// <summary>
104-
/// The number of content items to retrieve.
105-
/// </summary>
106-
private long _totalCount = -1;
107-
108-
#endregion parameter data
109-
11073
#region Command code
11174

11275
/// <summary>
@@ -116,15 +79,15 @@ protected override void ProcessRecord()
11679
{
11780
// TotalCount and Tail should not be specified at the same time.
11881
// Throw out terminating error if this is the case.
119-
if (_totalCountSpecified && _tailSpecified)
82+
if (TotalCount != -1 && Tail != -1)
12083
{
12184
string errMsg = StringUtil.Format(SessionStateStrings.GetContent_TailAndHeadCannotCoexist, "TotalCount", "Tail");
12285
ErrorRecord error = new(new InvalidOperationException(errMsg), "TailAndHeadCannotCoexist", ErrorCategory.InvalidOperation, null);
12386
WriteError(error);
12487
return;
12588
}
12689

127-
if (TotalCount == 0)
90+
if (TotalCount == 0 || Tail == 0)
12891
{
12992
// Don't read anything
13093
return;
@@ -141,23 +104,21 @@ protected override void ProcessRecord()
141104
{
142105
long countRead = 0;
143106

144-
Dbg.Diagnostics.Assert(
145-
holder.Reader != null,
146-
"All holders should have a reader assigned");
107+
Dbg.Diagnostics.Assert(holder.Reader != null, "All holders should have a reader assigned");
147108

148-
if (_tailSpecified && holder.Reader is not FileSystemContentReaderWriter)
109+
if (Tail != -1 && holder.Reader is not FileSystemContentReaderWriter)
149110
{
150111
string errMsg = SessionStateStrings.GetContent_TailNotSupported;
151112
ErrorRecord error = new(new InvalidOperationException(errMsg), "TailNotSupported", ErrorCategory.InvalidOperation, Tail);
152113
WriteError(error);
153114
continue;
154115
}
155116

156-
// If Tail is negative, we are supposed to read all content out. This is same
117+
// If Tail is -1, we are supposed to read all content out. This is same
157118
// as reading forwards. So we read forwards in this case.
158119
// If Tail is positive, we seek the right position. Or, if the seek failed
159120
// because of an unsupported encoding, we scan forward to get the tail content.
160-
if (Tail >= 0)
121+
if (Tail > 0)
161122
{
162123
bool seekSuccess = false;
163124

@@ -197,72 +158,61 @@ protected override void ProcessRecord()
197158
}
198159
}
199160

200-
if (TotalCount != 0)
161+
IList results = null;
162+
163+
do
201164
{
202-
IList results = null;
165+
long countToRead = ReadCount;
203166

204-
do
167+
// Make sure we only ask for the amount the user wanted
168+
// I am using TotalCount - countToRead so that I don't
169+
// have to worry about overflow
170+
if (TotalCount > 0 && (countToRead == 0 || TotalCount - countToRead < countRead))
205171
{
206-
long countToRead = ReadCount;
172+
countToRead = TotalCount - countRead;
173+
}
207174

208-
// Make sure we only ask for the amount the user wanted
209-
// I am using TotalCount - countToRead so that I don't
210-
// have to worry about overflow
175+
try
176+
{
177+
results = holder.Reader.Read(countToRead);
178+
}
179+
catch (Exception e) // Catch-all OK. 3rd party callout
180+
{
181+
ProviderInvocationException providerException =
182+
new(
183+
"ProviderContentReadError",
184+
SessionStateStrings.ProviderContentReadError,
185+
holder.PathInfo.Provider,
186+
holder.PathInfo.Path,
187+
e);
211188

212-
if ((TotalCount > 0) && (countToRead == 0 || (TotalCount - countToRead < countRead)))
213-
{
214-
countToRead = TotalCount - countRead;
215-
}
189+
// Log a provider health event
190+
MshLog.LogProviderHealthEvent(this.Context, holder.PathInfo.Provider.Name, providerException, Severity.Warning);
191+
WriteError(new ErrorRecord(providerException.ErrorRecord, providerException));
216192

217-
try
218-
{
219-
results = holder.Reader.Read(countToRead);
220-
}
221-
catch (Exception e) // Catch-all OK. 3rd party callout
193+
break;
194+
}
195+
196+
if (results != null && results.Count > 0)
197+
{
198+
countRead += results.Count;
199+
if (ReadCount == 1)
222200
{
223-
ProviderInvocationException providerException =
224-
new(
225-
"ProviderContentReadError",
226-
SessionStateStrings.ProviderContentReadError,
227-
holder.PathInfo.Provider,
228-
holder.PathInfo.Path,
229-
e);
230-
231-
// Log a provider health event
232-
MshLog.LogProviderHealthEvent(
233-
this.Context,
234-
holder.PathInfo.Provider.Name,
235-
providerException,
236-
Severity.Warning);
237-
238-
WriteError(new ErrorRecord(
239-
providerException.ErrorRecord,
240-
providerException));
241-
242-
break;
201+
// Write out the content as a single object
202+
WriteContentObject(results[0], countRead, holder.PathInfo, currentContext);
243203
}
244-
245-
if (results != null && results.Count > 0)
204+
else
246205
{
247-
countRead += results.Count;
248-
if (ReadCount == 1)
249-
{
250-
// Write out the content as a single object
251-
WriteContentObject(results[0], countRead, holder.PathInfo, currentContext);
252-
}
253-
else
254-
{
255-
// Write out the content as an array of objects
256-
WriteContentObject(results, countRead, holder.PathInfo, currentContext);
257-
}
206+
// Write out the content as an array of objects
207+
WriteContentObject(results, countRead, holder.PathInfo, currentContext);
258208
}
259-
} while (results != null && results.Count > 0 && ((TotalCount < 0) || countRead < TotalCount));
260-
}
209+
}
210+
} while (results != null && results.Count > 0 && (TotalCount == -1 || countRead < TotalCount));
261211
}
262212
}
263213
finally
264214
{
265-
// close all the content readers
215+
// Close all the content readers
266216

267217
CloseContent(contentStreams, false);
268218

@@ -284,7 +234,7 @@ private bool ScanForwardsForTail(in ContentHolder holder, CmdletProviderContext
284234
{
285235
var fsReader = holder.Reader as FileSystemContentReaderWriter;
286236
Dbg.Diagnostics.Assert(fsReader != null, "Tail is only supported for FileSystemContentReaderWriter");
287-
var tailResultQueue = new Queue<object>();
237+
Queue<object> tailResultQueue = new();
288238
IList results = null;
289239
ErrorRecord error = null;
290240

@@ -327,7 +277,10 @@ private bool ScanForwardsForTail(in ContentHolder holder, CmdletProviderContext
327277
foreach (object entry in results)
328278
{
329279
if (tailResultQueue.Count == Tail)
280+
{
330281
tailResultQueue.Dequeue();
282+
}
283+
331284
tailResultQueue.Enqueue(entry);
332285
}
333286
}
@@ -349,21 +302,25 @@ private bool ScanForwardsForTail(in ContentHolder holder, CmdletProviderContext
349302
{
350303
// Write out the content as single object
351304
while (tailResultQueue.Count > 0)
305+
{
352306
WriteContentObject(tailResultQueue.Dequeue(), count++, holder.PathInfo, currentContext);
307+
}
353308
}
354309
else // ReadCount < Queue.Count
355310
{
356311
while (tailResultQueue.Count >= ReadCount)
357312
{
358-
var outputList = new List<object>((int)ReadCount);
313+
List<object> outputList = new((int)ReadCount);
359314
for (int idx = 0; idx < ReadCount; idx++, count++)
315+
{
360316
outputList.Add(tailResultQueue.Dequeue());
317+
}
318+
361319
// Write out the content as an array of objects
362320
WriteContentObject(outputList.ToArray(), count, holder.PathInfo, currentContext);
363321
}
364322

365-
int remainder = tailResultQueue.Count;
366-
if (remainder > 0)
323+
if (tailResultQueue.Count > 0)
367324
{
368325
// Write out the content as an array of objects
369326
WriteContentObject(tailResultQueue.ToArray(), count, holder.PathInfo, currentContext);

‎test/powershell/Modules/Microsoft.PowerShell.Management/Get-Content.Tests.ps1‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ Describe "Get-Content" -Tags "CI" {
7272
Get-Content -Path $testPath2 -Last 1 | Should -BeExactly $fifthline
7373
}
7474

75+
It 'Verifies -TotalCount reports a ParameterArgumentValidationError error for negative values' {
76+
{Get-Content -Path $testPath2 -TotalCount -2} | Should -Throw -ErrorId 'ParameterArgumentValidationError,Microsoft.PowerShell.Commands.GetContentCommand'
77+
}
78+
79+
It 'Verifies -Tail reports a ParameterArgumentValidationError error for negative values' {
80+
{Get-Content -Path $testPath2 -Tail -2} | Should -Throw -ErrorId 'ParameterArgumentValidationError,Microsoft.PowerShell.Commands.GetContentCommand'
81+
}
82+
7583
It "Should be able to get content within a different drive" {
7684
Push-Location env:
7785
$expectedoutput = [Environment]::GetEnvironmentVariable("PATH");
@@ -271,6 +279,10 @@ Describe "Get-Content" -Tags "CI" {
271279
Get-Content -Path $testPath -TotalCount 0 | Should -BeNullOrEmpty
272280
}
273281

282+
It "Should return no content when -Tail value is 0" {
283+
Get-Content -Path $testPath -Tail 0 | Should -BeNullOrEmpty
284+
}
285+
274286
It "Should throw TailAndHeadCannotCoexist when both -Tail and -TotalCount are used" {
275287
{
276288
Get-Content -Path $testPath -Tail 1 -TotalCount 1 -ErrorAction Stop

0 commit comments

Comments
 (0)