View Issue Details

IDProjectCategoryView StatusLast Update
0031514Open CASCADEOCCT:Foundation Classespublic2021-09-05 23:28
Reportermsv Assigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version7.4.0 
Target Version7.6.0Fixed in Version7.6.0 
Summary0031514: Foundation Classes - Add Base64 encoding function
DescriptionWe have FSD_Base64Decoder::Decode() that is to decode Base64 data.
It is needed to implement encoding function.
Probably, we need to rename FSD_Base64Decoder to FSD_Base64.
Steps To ReproduceN/A
TagsNo tags attached.
Test case numberNot required

Relationships

related to 0031513 closedbugmaster Data Exchange - FSD_Base64Decoder::Decode() returns buffer with wrong length 
child of 0030692 closedbugmaster Data Exchange - introduce base framework RWMesh for importing mesh data formats into XDE document 

Activities

git

2020-04-23 13:12

administrator   ~0091727

Branch CR31514-ASRV has been created by msv.

SHA-1: 3d6ec4c72e5b661ea9206c575ee72ba8844bd985


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 13:13:11 2020 +0300

    0031514: Foundation Classes - Add Base64 encoding function
    
    The class FSD_Base64Decoder has been renamed to FSD_Base64.
    The new method FSD_Base64::Encode has been added.

msv

2020-04-23 13:13

developer   ~0091729

Dear Kirill, could you review the branch CR31514-ASRV?

git

2020-04-23 13:26

administrator   ~0091731

Branch CR31514-ASRV has been updated forcibly by msv.

SHA-1: 315cb2a783015bd8d7edf34af8d0a5bb8a4b27d4

git

2020-04-23 13:28

administrator   ~0091732

Branch CR31514 has been created by msv.

SHA-1: ddb4e6eacc88350abc4e976c1ab6887790f56444


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 13:13:11 2020 +0300

    0031514: Foundation Classes - Add Base64 encoding function
    
    The class FSD_Base64Decoder has been renamed to FSD_Base64.
    The new method FSD_Base64::Encode has been added.

msv

2020-04-23 13:32

developer   ~0091733

Tests are running.
http://jenkins-test-12.nnov.opencascade.com/view/CR31514-master-MSV/view/COMPARE/

git

2020-04-23 14:04

administrator   ~0091734

Branch CR31514 has been updated forcibly by msv.

SHA-1: dd66989098f289c7ec228f0b9f29cce26e29d368

git

2020-04-23 15:31

administrator   ~0091736

Branch CR31514-ASRV has been updated forcibly by msv.

SHA-1: 03101662cc289bdad04c193b94bb013c4d1d0a11

git

2020-04-23 17:50

administrator   ~0091737

Branch CR31514-ASRV has been updated by msv.

SHA-1: c7a28bc0e089b4e717e1b229e3c4787613e48818


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 17:51:41 2020 +0300

    #Change API

git

2020-04-23 18:05

administrator   ~0091740

Branch CR31514_1 has been created by msv.

SHA-1: 7d909ac7b4a11cbe61007c18a1d8d900566f7ee3


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 13:13:11 2020 +0300

    0031514: Foundation Classes - Add Base64 encoding function
    
    The class FSD_Base64Decoder has been renamed to FSD_Base64.
    The new method FSD_Base64::Encode has been added.

git

2020-04-23 18:08

administrator   ~0091741

Branch CR31514-ASRV has been updated forcibly by msv.

SHA-1: c293fd6a9c31919e8e9114a25604e097754d5d3c

git

2020-04-23 18:54

administrator   ~0091742

Branch CR31514_1 has been updated by msv.

SHA-1: 839c2da334f13276f6c7187fd11a96d3b1a2f9e3


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 18:55:27 2020 +0300

    #Fix regression

git

2020-04-23 18:57

administrator   ~0091743

Branch CR31514-ASRV has been updated forcibly by msv.

SHA-1: dd9f370110e5b0a231e2ff80cf3c0728ec1e0f7c

msv

2020-04-23 19:01

developer   ~0091744

Please review again.
Tests are running.
http://jenkins-test-12.nnov.opencascade.com/view/CR31514-master-MSV/view/COMPARE/

msv

2020-04-23 19:01

developer   ~0091745

The branch is CR31514_1.

kgv

2020-04-23 19:05

developer   ~0091746

No more remarks - please raise the patch in OCCT branch CR31514_1.

git

2020-04-23 19:13

administrator   ~0091748

Branch CR31514-ASRV has been updated by msv.

SHA-1: 400f8b49dd4e502188864602e601dc14e17ecb65


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 19:14:06 2020 +0300

    #Eliminate compilation warning on clang

git

2020-04-23 19:18

administrator   ~0091749

Branch CR31514_1 has been updated by msv.

SHA-1: 68ac72fb9d23a73a3e16ffd94cc9c700c0f158af


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 19:14:06 2020 +0300

    #Eliminate compilation warning on clang

git

2020-04-23 19:29

administrator   ~0091750

Branch CR31514-ASRV has been updated forcibly by msv.

SHA-1: 7d6964bb7d92225d19272d19ef1e5b346719e397

bugmaster

2020-04-24 10:40

administrator   ~0091766

Combination -
OCCT branch : WEEK-17
master SHA - 03fce6fac0f286c86b6b5c49a43b094fc4a8ab07
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : WEEK-17 SHA - 30cd52f3d9c1f4ceaee9b46e90331b1e68a80d57
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17149.45000000016 / 16936.94000000008 [+1.25%]
Products
Total CPU difference: 11292.350000000108 / 11283.690000000102 [+0.08%]
Windows-64-VC14:
OCCT
Total CPU difference: 18622.171875 / 18389.65625 [+1.26%]
Products
Total CPU difference: 13047.59375 / 13180.625 [-1.01%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2020-04-26 11:46

administrator   ~0091795

Branch CR31514_1 has been updated forcibly by msv.

SHA-1: 1fd51cf84255a8cc6e29e972ebfd9f841704931b

msv

2020-04-26 11:48

developer   ~0091796

I have made agreed corrections. Please review.

git

2020-04-27 11:07

administrator   ~0091818

Branch CR31514_2 has been created by msv.

SHA-1: 97d4c8dc172cd7ddce2e4ce8f73a511c5b28ca62


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 13:13:11 2020 +0300

    0031514: Foundation Classes - Add Base64 encoding function
    
    The class FSD_Base64Decoder has been renamed to FSD_Base64.
    The new method FSD_Base64::Encode has been added.
    The method Decode has been improved for performance.

msv

2020-04-27 11:09

developer   ~0091819

Please review again.

abv

2020-04-28 22:16

manager   ~0091828

Please consider remarks:

1. Functions Encode() and Decode() take pointer to buffer of unspecified size and use it assuming implicitly that it will be sufficient for the data. There is no check for this, and that is quite dangerous. Please consider taking NCollection_Buffer or TColStd_(H)Array1OfByte (which both can either allocate their own buffer or attach to existing one).

2. Please follow OCCT conventions for names of local variables. I would suggest renaming "B64Map" to "aBase64Chars" and "B64Codes" to "aBase64Codes"

3. Please use consistent types: either Standard_Integer or int (see lines 46 vs 56). I would suggest using types with known size, such as int32_t (or int16_t, if appropriate), uint16_t etc. (size of int may differ between platforms)

4. In second FSD_Base64::Decode(), I believe generation of error message should be avoided - it is too low-level function to generate user messages directly. In case of insufficient memory the caller will know that by receiving null handle (this is the only case, null return for empty input buffer described in header is apparently wrong), and can generate the message as needed.

5. Please consider providing second implementation of Encode() returning NCollection_Buffer, for consistency with Decode()

msv

2020-04-29 11:52

developer   ~0091846

1. Usage of NCollection_Buffer is inconvenient due to necessity to provide the allocator. I think it is better for such low-level method not to bind to any complex OCCT types. I will rather add the new length parameter for safety.

2. OK

3. OK

4. OK to get rid of generation of message. However, in the case of empty input buffer the method will return null. It is because NCollection_Buffer::Allocate will return 0 if requested size is 0.

By the way, there is strange code in that method:
    if (theSize != 0
    || !myAllocator.IsNull())
    {
      myData = (Standard_Byte* )myAllocator->Allocate (theSize);
    }
So, if theSize != 0 it will call myAllocator->Allocate even if myAllocator is null.

5. I will provide Encode() returning TCollection_AsciiString, as it is natural to use it rather than NCollection_Buffer for storing strings.

msv

2020-04-29 12:10

developer   ~0091847

It would be good to create test cases for coding/decoding Base64.

git

2020-04-29 12:14

administrator   ~0091848

Branch CR31514_2 has been updated by msv.

SHA-1: 3d80d4e6719bdce881577a5a1a303d98e489d161


Detailed log of new commits:

Author: msv
Date: Wed Apr 29 12:14:29 2020 +0300

    #considering remarks

msv

2020-04-29 12:14

developer   ~0091849

Please review latest changes. I will add tests later.

abv

2020-04-29 12:28

manager   ~0091850

I agree that NCollection_Buffer is ugly, but that is a chance to improve it:
- Use default OCCT Allocator if no one is provided
- Correctly treat externally provided buffer (either do not return it, or do that on explicit request)
- Either prohibit or implement correctly copy construction and assignment

I believe TCollection_AsciiString is better not to be used - here we actually work with byte buffer rather than with text string, and will likely need to access the data in a manner not directly supported by that class (such as get plain pointer).

On 4: I believe the difference is between returning Null NCollection_Buffer and returning NCollection_Buffer with Null data.

abv

2020-04-29 12:35

manager   ~0091853

Besides, should not we have explicit methods to compute size for target buffer instead of using cryptic approach with calling main method with incomplete parameters? We do not develop for WinAPI where this approach was adopted.

msv

2020-04-29 13:02

developer   ~0091855

Improvement of NCollection_Buffer is out of scope of this bug. If needed please add a new bug. And I probably won't have time to do in that direction.

TCollection_AsciiString is good for working with strings, and it has the methods to work with its content as a buffer: ctor with number of chars, and ToCString(). The latter returns const char*, and we can add a new method like std::string::data() to get access to non-const content.

(4) I did not understand your idea. What should the method return in which case?

The idea to use WinAPI manner was sounded by Kirill, and I followed his recommendations.

kgv

2020-04-29 13:15

developer   ~0091857

> I agree that NCollection_Buffer is ugly, but that is a chance to improve it:
> - Use default OCCT Allocator if no one is provided
I suppose you mean adding default NCollection_Buffer constructor which will use Common allocator. Passing NULL to existing constructor should be kept as is, or it will break existing code wrapping external data by passing NULL allocator.

> - Either prohibit or implement correctly copy construction and assignment
I suppose copy/assignment should be disabled.

> Besides, should not we have explicit methods to compute size
Current approach was proposed not for mimicking WinAPI (it is not only WinAPI, by the way) but to provide clear interface with single entry instead of defining numerous methods with potentially tangled or duplicated logic (probably not that critical in case of base64 codec). I found this design elegant and commonly known.

abv

2020-04-30 17:23

manager   ~0091913

@msv: 4) I did not understand your idea. What should the method return in which case?

My point was only that comment to 2nd method Decode in header file is not correct:

//! @return null handle if theLen is 0 or in case of out of memory condition.

Actual implementation is that if theLen is 0, you will get non-null handle to NCollection_Buffer object of null size, this is not the same as null handle (returned in case of memory allocation failure).

msv

2020-04-30 18:02

developer   ~0091914

Actually, NCollection_Buffer::Allocate() returns false in case of the requested size is 0. So, all in all null handle will be returned.

abv

2020-05-01 22:29

manager   ~0091925

I see your point, but still deem you are wrong. Please have a closer look at NCollection_Buffer::Allocate(): it has a bug in the "if" condition:

    if (theSize != 0
    || !myAllocator.IsNull())

Here we have || instead of &&, this means that for theSize == 0 allocator will be called (as soon as it is not null) and return non-zero buffer (note that allocator shall return non-zero pointer for requested block of zero size, following behavior of malloc()). Hence, the allocation shall succeed and return true.

msv

2020-05-02 12:16

developer   ~0091926

I have already told about this bug above: see 0031514:0091846.
When it is fixed the description will be true ;)
Do you think the method Decode must return non-null empty buffer in case of input zero-size string?

abv

2020-05-03 07:25

manager   ~0091929

Yes I do - it is commonly adopted behavior that empty buffer is valid object of 0 size, not a NULL object.

git

2021-08-24 00:06

administrator   ~0103381

Branch CR31514_3 has been created by kgv.

SHA-1: c3a4418696a5cbb67daf3fa042ed75852bed3858


Detailed log of new commits:

Author: msv
Date: Thu Apr 23 13:13:11 2020 +0300

    0031514: Foundation Classes - Add Base64 encoding function
    
    The class FSD_Base64Decoder has been renamed to FSD_Base64.
    The new method FSD_Base64::Encode has been added.
    The method Decode has been improved for performance.

kgv

2021-08-24 01:34

developer   ~0103401

Please raise the patch
- OCCT: branch CR31514_3.

http://jenkins-test-12.nnov.opencascade.com:8080/view/CR31514_3-master-KGV/

bugmaster

2021-08-28 15:43

administrator   ~0103470

Combination -
OCCT branch : IR-2021-08-27
master SHA - 8f70959571ab8999b47f88d673343587e733f364
a87b7ddc8cb44606b91e3f37113847c3f5f50fdc
Products branch : IR-2021-08-27 SHA - 71e8b69563894a6e7348c9bd5b0294a5e5e8fe3f
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17432.060000000307 / 17364.750000000335 [+0.39%]
Products
Total CPU difference: 11534.270000000102 / 11550.27000000011 [-0.14%]
Windows-64-VC14:
OCCT
Total CPU difference: 19189.15625 / 19243.171875 [-0.28%]
Products
Total CPU difference: 12874.53125 / 12885.734375 [-0.09%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2021-08-28 15:59

administrator   ~0103477

Branch CR31514 has been deleted by mnt.

SHA-1: dd66989098f289c7ec228f0b9f29cce26e29d368

git

2021-08-28 15:59

administrator   ~0103478

Branch CR31514_1 has been deleted by mnt.

SHA-1: 1fd51cf84255a8cc6e29e972ebfd9f841704931b

git

2021-08-28 15:59

administrator   ~0103479

Branch CR31514_2 has been deleted by mnt.

SHA-1: 3d80d4e6719bdce881577a5a1a303d98e489d161

git

2021-08-28 15:59

administrator   ~0103480

Branch CR31514_3 has been deleted by mnt.

SHA-1: c3a4418696a5cbb67daf3fa042ed75852bed3858

git

2021-09-05 23:28

administrator   ~0103839

Branch CR31514-ASRV has been deleted by kgv.

SHA-1: 7d6964bb7d92225d19272d19ef1e5b346719e397

Related Changesets

occt: master 45d498ef

2020-04-23 10:13:11

msv


Committer: bugmaster Details Diff
0031514: Foundation Classes - Add Base64 encoding function

The class FSD_Base64Decoder has been renamed to FSD_Base64.
The new method FSD_Base64::Encode has been added.
The method Decode has been improved for performance.
Affected Issues
0031514
mod - src/FSD/FILES Diff File
add - src/FSD/FSD_Base64.cxx Diff File
add - src/FSD/FSD_Base64.hxx Diff File
rm - src/FSD/FSD_Base64Decoder.cxx Diff File
rm - src/FSD/FSD_Base64Decoder.hxx Diff File
mod - src/RWGltf/RWGltf_GltfJsonParser.cxx Diff File

Issue History

Date Modified Username Field Change
2020-04-23 10:09 msv New Issue
2020-04-23 10:09 msv Assigned To => msv
2020-04-23 10:21 kgv Relationship added child of 0030692
2020-04-23 13:12 git Note Added: 0091727
2020-04-23 13:13 msv Note Added: 0091729
2020-04-23 13:26 git Note Added: 0091731
2020-04-23 13:28 git Note Added: 0091732
2020-04-23 13:32 msv Note Added: 0091733
2020-04-23 13:33 msv Status new => assigned
2020-04-23 13:33 msv Assigned To msv => kgv
2020-04-23 13:33 msv Status assigned => resolved
2020-04-23 13:33 msv Steps to Reproduce Updated
2020-04-23 14:04 git Note Added: 0091734
2020-04-23 15:31 git Note Added: 0091736
2020-04-23 17:50 git Note Added: 0091737
2020-04-23 18:05 git Note Added: 0091740
2020-04-23 18:08 git Note Added: 0091741
2020-04-23 18:54 git Note Added: 0091742
2020-04-23 18:57 git Note Added: 0091743
2020-04-23 19:01 msv Note Added: 0091744
2020-04-23 19:01 msv Note Added: 0091745
2020-04-23 19:05 kgv Note Added: 0091746
2020-04-23 19:05 kgv Assigned To kgv => bugmaster
2020-04-23 19:05 kgv Status resolved => reviewed
2020-04-23 19:13 git Note Added: 0091748
2020-04-23 19:18 git Note Added: 0091749
2020-04-23 19:29 git Note Added: 0091750
2020-04-24 10:40 bugmaster Note Added: 0091766
2020-04-24 10:40 bugmaster Status reviewed => tested
2020-04-24 10:43 bugmaster Test case number => Not required
2020-04-24 20:05 bugmaster Assigned To bugmaster => abv
2020-04-24 20:05 bugmaster Status tested => assigned
2020-04-26 11:46 git Note Added: 0091795
2020-04-26 11:48 msv Note Added: 0091796
2020-04-26 11:48 msv Status assigned => resolved
2020-04-27 09:41 msv Assigned To abv => msv
2020-04-27 09:41 msv Status resolved => assigned
2020-04-27 11:07 git Note Added: 0091818
2020-04-27 11:09 msv Note Added: 0091819
2020-04-27 11:09 msv Assigned To msv => abv
2020-04-27 11:09 msv Status assigned => resolved
2020-04-28 22:16 abv Note Added: 0091828
2020-04-28 22:16 abv Assigned To abv => msv
2020-04-28 22:16 abv Status resolved => assigned
2020-04-28 22:59 abv Relationship added related to 0031513
2020-04-29 11:52 msv Note Added: 0091846
2020-04-29 12:10 msv Note Added: 0091847
2020-04-29 12:14 git Note Added: 0091848
2020-04-29 12:14 msv Note Added: 0091849
2020-04-29 12:15 msv Assigned To msv => abv
2020-04-29 12:15 msv Status assigned => resolved
2020-04-29 12:28 abv Note Added: 0091850
2020-04-29 12:35 abv Note Added: 0091853
2020-04-29 12:35 abv Assigned To abv => msv
2020-04-29 12:35 abv Status resolved => feedback
2020-04-29 13:02 msv Note Added: 0091855
2020-04-29 13:02 msv Assigned To msv => abv
2020-04-29 13:15 kgv Note Added: 0091857
2020-04-30 17:23 abv Note Added: 0091913
2020-04-30 18:02 msv Note Added: 0091914
2020-05-01 22:29 abv Note Added: 0091925
2020-05-02 12:16 msv Note Added: 0091926
2020-05-03 07:25 abv Note Added: 0091929
2020-09-25 20:35 abv Target Version 7.5.0 => 7.6.0
2021-08-24 00:06 git Note Added: 0103381
2021-08-24 01:34 kgv Note Added: 0103401
2021-08-24 01:34 kgv Assigned To abv => bugmaster
2021-08-24 01:34 kgv Status feedback => reviewed
2021-08-28 15:43 bugmaster Note Added: 0103470
2021-08-28 15:43 bugmaster Status reviewed => tested
2021-08-28 15:51 bugmaster Changeset attached => occt master 45d498ef
2021-08-28 15:51 bugmaster Status tested => verified
2021-08-28 15:51 bugmaster Resolution open => fixed
2021-08-28 15:59 git Note Added: 0103477
2021-08-28 15:59 git Note Added: 0103478
2021-08-28 15:59 git Note Added: 0103479
2021-08-28 15:59 git Note Added: 0103480
2021-09-05 23:28 git Note Added: 0103839