View Issue Details

IDProjectCategoryView StatusLast Update
0037332FPCFCLpublic2020-07-15 11:00
ReporterBBaz Assigned ToMarco van de Voort  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.2.0 
Summary0037332: TJSONParser.Create(TStream, TJSONOptions) does not respect the position of the stream
DescriptionThis is a regression. Previously the stream passed as parameter was read from its current position, now it is fully loaded.
The problem is that the content going from 0 to Position() may contain data unrealted to JSON.
Steps To Reproduceprogram Project1;

uses Classes, jsonparser;

var
  s : TMemoryStream;
  p : TJSONParser;
  i : integer;
  t : string = 0000001#02#03#04'{}';program Project1;

uses Classes, jsonparser;

var
  s : TMemoryStream;
  p : TJSONParser;
  i : integer;
  t : string = 0000001#02#03#04'{}';
begin
  s := TMemoryStream.Create;
  try
  s.Write(t[1], length(t));
  s.Position := 0;
  s.Read(i, 4);
  p := TJSONParser.Create(s);
  try
    p.Parse();
  finally
    p.Free;
  end;
  finally
    s.Free;
  end;
end.
begin
  s := TMemoryStream.Create;
  try
  s.Write(t[1], length(t));
  s.Position := 0;
  s.Read(i, 4);
  p := TJSONParser.Create(s);
  try
    p.Parse();
  finally
    p.Free;
  end;
  finally
    s.Free;
  end;
end.
TagsNo tags attached.
Fixed in Revision45776
FPCOldBugId
FPCTarget-
Attached Files

Activities

BBaz

2020-07-11 20:32

reporter   ~0123892

take care in my example the link 0000001 was actually : sharp character followed by 01...

BBaz

2020-07-11 20:33

reporter   ~0123893

sorry, use this test case instead:

---
program Project1;

uses Classes, jsonparser;

var
  s : TMemoryStream;
  p : TJSONParser;
  i : integer;
  t : string = '^^^^{}';
begin
  s := TMemoryStream.Create;
  try
  s.Write(t[1], length(t));
  s.Position := 0;
  s.Read(i, 4);
  p := TJSONParser.Create(s);
  try
    p.Parse();
  finally
    p.Free;
  end;
  finally
    s.Free;
  end;
end.
---

Marco van de Voort

2020-07-12 14:36

manager   ~0123927

Last edited: 2020-07-12 14:39

View 3 revisions

I think the problem is

constructor TJSONScanner.Create(Source: TStream; AOptions: TJSONOptions);

in jsonscanner.pp, it puts the stream into a string, reading stream.size bytes, iow assuming the position=0.

Probably should be
  SetLength(S,Source.Size-Source.Position);

please test with that modification.

IMHO requiring the whole stream in memory before operating on it is bad design.

Marco van de Voort

2020-07-12 14:39

manager   ~0123928

See above

BBaz

2020-07-12 17:35

reporter   ~0123937

Yes this more or less what I did to fix: read correctly in a string at the call site and use the parser constructor overload that takes a string.


> IMHO requiring the whole stream in memory before operating on it is bad design.

The fact that I use a TMemoryStream is not relevant. This could be a FIleStream and the same bug would happen.

Sven Barth

2020-07-12 21:18

manager   ~0123940

I think Marco refers to the TJSONScanner copying the whole stream into a temporary buffer.

Also he'd like you to test your own code with the fix he mentioned applied to TJSONScanner to see whether it would be a correct fix.

Marco van de Voort

2020-07-12 21:21

manager   ~0123942

Fixed. Btw I meant that reading streams, and converting the string to stream would be better than the other way around. But that would be a too big rewrite right now.

BBaz

2020-07-15 11:00

reporter   ~0124031

thanks for the quick reaction.

Issue History

Date Modified Username Field Change
2020-07-11 20:28 BBaz New Issue
2020-07-11 20:32 BBaz Note Added: 0123892
2020-07-11 20:33 BBaz Note Added: 0123893
2020-07-12 14:36 Marco van de Voort Note Added: 0123927
2020-07-12 14:38 Marco van de Voort Note Edited: 0123927 View Revisions
2020-07-12 14:39 Marco van de Voort Note Edited: 0123927 View Revisions
2020-07-12 14:39 Marco van de Voort Assigned To => Marco van de Voort
2020-07-12 14:39 Marco van de Voort Status new => feedback
2020-07-12 14:39 Marco van de Voort FPCTarget => -
2020-07-12 14:39 Marco van de Voort Note Added: 0123928
2020-07-12 17:35 BBaz Note Added: 0123937
2020-07-12 17:35 BBaz Status feedback => assigned
2020-07-12 21:18 Sven Barth Note Added: 0123940
2020-07-12 21:21 Marco van de Voort Status assigned => resolved
2020-07-12 21:21 Marco van de Voort Resolution open => fixed
2020-07-12 21:21 Marco van de Voort Fixed in Revision => 45776
2020-07-12 21:21 Marco van de Voort Note Added: 0123942
2020-07-15 11:00 BBaz Status resolved => closed
2020-07-15 11:00 BBaz Note Added: 0124031