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.

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

Range check error on POST request on Linux #81

Closed
BlackLanzer opened this issue Mar 2, 2020 · 6 comments
Closed

Range check error on POST request on Linux #81

BlackLanzer opened this issue Mar 2, 2020 · 6 comments

Comments

@BlackLanzer
Copy link

Describe the bug
If you send an array in POST on Linux the server crash with Range check error on this line https://github.com/andrea-magni/MARS/blob/master/Source/MARS.Rtti.Utils.pas#L255.
On Windows it works fine.

To Reproduce
Steps to reproduce the behavior:

  1. Send an array in POST

Expected behavior
The program should not crash

Scenario (please complete the following information):

  • OS: Linux
@meg02in
Copy link
Contributor

meg02in commented Apr 20, 2020

I have the same problem in my project. I fixed the issue modifing the procedure in that way:

//procedure SetArrayLength(var AArray: TValue; const AArrayType: TRttiType; const ANewSize: Integer);
procedure SetArrayLength(var AArray: TValue; const AArrayType: TRttiType; const ANewSize: PNativeInt);
begin
  if AArrayType is TRttiArrayType then
  begin
    raise Exception.Create('Not yet implemented: SetArrayLength TRttiArrayType');
    { TODO -oAndrea : probably not needed }
  end
  else if AArrayType is TRttiDynamicArrayType then
    //DynArraySetLength(PPointer(AArray.GetReferenceToRawData)^, AArrayType.Handle, 1, @ANewSize);
    DynArraySetLength(PPointer(AArray.GetReferenceToRawData)^, AArrayType.Handle, 1, ANewSize);
end;

and the changing accordly the unit MARS.Core.JSON.pas and MARS.Core.MessageBodyReaders.pas.

After that my project working good both under linux and under wndows in all kind of deploying method for the server tha MARS support.

I attached my source of MARS.Rtti.Utils, MARS.Core.JSON.pas and MARS.Core.MessageBodyReaders.pas

RangeCheckFixing.zip

@BlackLanzer
Copy link
Author

BlackLanzer commented Apr 26, 2020

Nice find @meg02in, but your fix has another bug.

In MARS.Core.MessageBodyReaders.pas and MARS.Core.JSON.pas you define the variable L for the new size of the array as an Integer.
Later DynArraySetLength take a pointer to NativeInt. In my tests this caused a wrong read of the pointer value. I think it's because of 64 or 32 bit stuff.

I fixed it defining L as NativeInt and now it's working correctly on both Windows and Linux.

I can make a pull request later if everything is good.

@meg02in
Copy link
Contributor

meg02in commented May 8, 2020

Hi,
thanks for your suggestion, i made your correction and tested for 1 week in a server of one of my customer.

All gone right

@andrea-magni
Copy link
Owner

Hi @meg02in and @BlackLanzer , please go ahead and submit your PR! Thanks!
Please beware I have updated a bit the develop branch today so you may want to make a Pull before proceeding.

Waiting for your PR :-)

@andrea-magni
Copy link
Owner

For your convenience, I've just merged your corrections in the attached source files.
Please check everything is as you like and submit your PR, I will be happy to accept it.
If you could just rename that L variable to something like LNewLength I would appreciated it.

Thanks again :-)

Source.zip

meg02in added a commit to meg02in/MARS that referenced this issue May 22, 2020
Correction for Range check error on POST request on Linux
andrea-magni#81
andrea-magni added a commit that referenced this issue May 27, 2020
Correction for Range check error on POST request on Linux
#81

Co-authored-by: Andrea Magni <andrea.magni@gmail.com>
andrea-magni added a commit that referenced this issue May 27, 2020
* TXMLReader and TXMLWriter implementation (MBR and MBW mechanism for IXMLDocument and IXMLNode)

* DataSetToXML now has a <dataset> root node

* Added shortcut class methods WriteDataSet for TDataSetWriterJSON and TDataSetWriterXML classes

* IMPORTANT: Changes in the MessageBodyWriter selection mechanism.

Affinity now comes with higher priority than QualityFactor. In other words, this means server side type matching comes before client side accept statement.
Improved TMediaTypeList.Intersect (now properly considering WILDCARD also).
Improved TMediaTypeList.GetQualityFactor (now properly considering WILDCARD also).
TFDDataSetWriter is now decorated with Produces attributes only for APPLICATION_JSON_FIREDAC, APPLICATION_XML_FIREDAC and APPLICATION_OCTET_STREAM (removed APPLICATION_XML). This will let TDataSetWriterXML to jump in if registered.
Added WriteDataSet shortcut method to TArrayFDDataSetWriter class.
Refactoring.

* TMARSFireDAC.LoadConnectionDefs now checks if a Connection Definition is already available before defining it. This means you can have a connection definition on your development machine (configured through FireDAC standard mechanisms such as FireDAC Explorer or ini files) and at the same time include a definition for deployment or for development machines where design time experience is not required.
BEWARE: no exception is thrown so keep in mind a local definition (FireDAC Explorer / ini files) actually overrides what's defined in MARS Engine parameters/ini files.

* Bugfix: TMARSFireDAC.InjectMacroValues and InjectParamValues assumed ACommand always assigned (but for example a TFDMemTable has Command = nil). Thanks Stuart Clennet for pointing this out.

* Added FindWriter overload for specific Accept lookup, new TMARSMessageBodyWriter class (utility functions container for MBW)

* Bugfix: TMARSNetClient implementations for LastCmdSuccess, ResponseStatusCode and ResponseText raised an exception if the FLastResponse object was not assigned. This can happen when connection troubles occur (thanks Stuart Clennet for pointing this out in this thread https://en.delphipraxis.net/topic/436-error-when-accessing-tmarsnetclient-responsestatuscode-when-endpoint-is-blacklisted/ )

* Bugifx #53: packages for 10.3 Rio are now correct

* Bugfix #54: fixed TStringReader, added Encoding support (initial) for MBR and refactoring.

* Bugfix #57: TJSONObjectHelper.ReadDateTimeValue ignores ADefault argument

* Introduction of IMARSRequest and IMARSResponse
Wrappers for TWebRequest and TWebResponse

* * resolving conflicts to merge Brov84-develop

* Added x-www-form-urlencoded support for MARS Resources (#56)

* Added x-www-form-urlencoded for MARS Client Resources

* Refactoring MARS URLEncoded form support
Register new resource component
Packages 10.3 Rio for URLEncoded form support
MARS.Core.RequestAndResponse.Interfaces now added to MARS Utils package

* + Delphi-Cross-Socket https://github.com/winddriver/Delphi-Cross-Socket

* TMARSWebRequest and TMARSWebResponse wrap for Server.WebModule.pas

* Removed hint for missing inline expansion

* + MARS.DCS package
* updated packages for 10.3

* BugFix: missing FireBeforeExecute in TMARSCustomClient (preventing anonyms added with RegisterBeforeExecute to fire when using regular component based calls, it was working for non-component based approach).
Thanks @Davide675 for spotting this bug.

* BugFix: missing FireBeforeExecute in TMARSCustomClient (preventing anonyms added with RegisterBeforeExecute to fire when using regular component based calls, it was working for non-component based approach).
Thanks @Davide675 for spotting this bug.

# Conflicts:
#	Source/MARS.Client.Client.pas

* BugFix: wrong default for configuration file name when deployed as Apache module. Fixes #67, thanks @acarlomagno

* Add Linux Server Delphi 10.2, 10.3 (#68)

Great thanks!

* Further refactoring to abstract from Web.HttpApp and Indy http layer
BugFix: changes about RequiredAttribute in commit a0bcbce broke handling of multipart/form-data files (Demos/MultipartFormData), thanks @stuartclennett for pointing this out (https://en.delphipraxis.net/topic/243-handling-multipartformdata/)
Refactoring

* Add {$IFDEF TESTINSIGHT} (#69)

Thanks @tino-t :-)

* MARS.mORMot Its not suported for Linux64 (#71)

MARS.FireDAC suported for Linux64
MARS.JOSE suported for Linux64

* Updated Mustache demo to use IMARSResponse and IMARSRequest. Enabled CORS and added StatusCode for swagger endpoint. (#70)

* JOSE JWT upgrade to latest version

* Upgrade Delphi-Cross-Socket to the latest version

* Fix JOSE packages
Compilation issue with JOSEJWT
JWT tests are now using a 32 char length dummy secret

* Upgrade mORMot-JWT to the latest version

* Authorization demo fixes:
* dproj upgrade
* compilation issues with BeforeHandleRequest handler

* Set FDManager.SilentMode to True to avoid including Wait providers for FireDAC everywhere.

* Added GetQueryParamCount and GetCookieParamCount to IMARSRequest

* GitHub Social template for MARS

* TFDDataSetWriter now defaults to JSON serializer when the match is with the Wildcard media type (before the first entry in TFDDataSetWriter.WriteTo method would apply, that was the XML serializer).

* Develop (#75)

* Updated Mustache demo to use IMARSResponse and IMARSRequest. Enabled CORS and added StatusCode for swagger endpoint.

* Added last file needed to be independent of a separate DMustache library.

* UniDAC support (initial commit): many thanks to ErtanK for the implementation.
Added MARS.UniDAC package
Added "UniDAC Basic" demo

* WIP UniDAC support

* UniDAC: connection definition handling (WIP)

* UniDAC support: better handling of connection definitions (caching at startup), optimized by-name access;
Added SQLite example in UniDAC Basic demo
UniDAC dataset serialization up to TMemDataSet (and TVirtualTable)
Fixed macro injection

* UniDAC support: a SQLite connection has been added to UniDAC Basic/bin/UniDACBasicServerApplication.ini and some equivalent code using FireDAC has been added in Server.Resources.pas

* + Missing MARSClient.CoreResource.rc for 103Rio package

* Added the ability to setup Windows Service Name and DisplayName using MARS parameters.
Just add:
ServiceName=YourServiceName
ServiceDisplayName=Your Service Display Name
to the ini file (or whatever parameter's storage system you are using).

* dproj upgrade Delphi 10.3.3 Rio

* Fix ApplyCustomHeaders restored after rebase of develop onto master branch

* Fix visiblity (caused warning)

* Removed MARS.UniDAC package from MARS.groupproj

* Develop (#78)

* Updated Mustache demo to use IMARSResponse and IMARSRequest. Enabled CORS and added StatusCode for swagger endpoint.

* Added last file needed to be independent of a separate DMustache library.

* Implemented support for gzip compression

* Reverted modifications to MARS.Core.Activation.

* Revert "Implemented support for gzip compression"

This reverts commit 57d5264.

* Implementation for gzip compression.

* Implementation for gzip compression.

Co-authored-by: Bjørn Larsen <bjorn.larsen1@km.kongsberg.com>

* Changed GZip compression default to False in MARSTemplate

* + MARSTemplateDCS demo

* MARSTemplateDCS compilation

* WIP MARSTemplateDCS

* Issue #80: thanks to @grzegorzdeyk for pointing this out: Default encoding should be last call in GetEncodingName

* + RemoteMic demo

* Upgrade dproj 10.3.3

* Upgrade 10.4 Sydney
Added packages for Delphi 10.4 Sydney
Removed hints and warnings compiling with 10.4 Sydney

* Upgrade Delphi 10.4 Sydney: Demos/MARSTemplate dprojs

* Upgrade Delphi 10.4 Sydney: Demos/MARSTemplateDCS dprojs

* Include definitions updated for Delphi 10.4 Sydney

* Output directory updated for Delphi 10.4 Sydney (270)

* Correction SetArrayLength (#83)

Correction for Range check error on POST request on Linux
#81

Co-authored-by: Andrea Magni <andrea.magni@gmail.com>

* Merge error after PR#83

Co-authored-by: Paolo Brovelli <brov84@gmail.com>
Co-authored-by: luiguik2 <luiguik2@gmail.com>
Co-authored-by: Tino-T <tteuber@gmail.com>
Co-authored-by: Bjørn Larsen <bjorn@labit.no>
Co-authored-by: Bjørn Larsen <bjorn.larsen1@km.kongsberg.com>
Co-authored-by: Emmanuel Nocentini <64004621+meg02in@users.noreply.github.com>
@andrea-magni
Copy link
Owner

This issue has been fixed thanks to the pull request #83
Many thanks for your contribution! :-)

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

No branches or pull requests

3 participants