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.

Sign up for GitHub

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

Jump to bottom

Some more tests for the kernel #2502

Merged
merged 7 commits into from
Jun 1, 2018
Merged

Some more tests for the kernel #2502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7e9a454
Add some more stream tests
fingolfin Jun 1, 2018
3e8e490
stringobj.c: fix typos, comments
fingolfin Jun 1, 2018
a8f47b6
stringobj.c: remove pointless CHANGED_BAG calls
fingolfin Jun 1, 2018
57bba1b
stringobj.c: stricter validation of EmptyString() args
fingolfin Jun 1, 2018
7d6ad1a
stringobj.c: remove unused Make{Imm,}String3
fingolfin Jun 1, 2018
246719c
stringobj.c: better/simpler argument validation
fingolfin Jun 1, 2018
b63512a
Add some tests for kernel functions in stringobj.c
fingolfin Jun 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 45 additions & 78 deletions src/stringobj.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,17 +198,17 @@ void LoadChar( Obj c )

/****************************************************************************
**
*F FuncEmptyString( <self>, <len> ) . . . . . . . empty string with space
*
* Returns an empty string, but with space for len characters preallocated.
*
*F FuncEmptyString( <self>, <len> ) . . . . . . . . empty string with space
**
** Returns an empty string, but with space for len characters preallocated.
**
*/
Obj FuncEmptyString( Obj self, Obj len )
{
Obj new;
while ( ! IS_INTOBJ(len) ) {
while ( ! IS_NONNEG_INTOBJ(len) ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Strictly speaking this is a bug fix preventing OOB access. Perhaps should be mentioned in the commit message. Not sure if we care about it for release notes - perhaps a generic entry "improved input validation for several kernel functions"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think some generic comment like that would be useful.

len = ErrorReturnObj(
"<len> must be an integer (not a %s)",
"<len> must be an non-negative integer (not a %s)",
(Int)TNAM_OBJ(len), 0L,
"you can replace <len> via 'return <len>;'" );
}
Expand All @@ -220,11 +220,11 @@ Obj FuncEmptyString( Obj self, Obj len )

/****************************************************************************
**
*F FuncShrinkAllocationString( <self>, <str> ) . . give back unneeded memory
*
* Shrinks the bag of <str> to minimal possible size (possibly converts to
* compact representation).
*
*F FuncShrinkAllocationString( <self>, <str> ) . . give back unneeded memory
**
** Shrinks the bag of <str> to minimal possible size (possibly converts to
** compact representation).
**
*/
Obj FuncShrinkAllocationString( Obj self, Obj str )
{
Expand Down Expand Up @@ -406,19 +406,22 @@ Obj FuncSTRING_SINTLIST (
* integers ? */

/* general code */
if (! IS_RANGE(val) ) {
if (! IS_PLIST(val)) {
while (!IS_RANGE(val) && !IS_PLIST(val)) {
again:
val = ErrorReturnObj(
"<val> must be a plain list or range, not a %s)",
"<val> must be a plain list of small integers or a range, not a %s",
(Int)TNAM_OBJ(val), 0L,
"you can replace <val> via 'return <val>;'" );
}
}
if (! IS_RANGE(val) ) {
l=LEN_PLIST(val);
n=NEW_STRING(l);
p=CHARS_STRING(n);
for (i=1;i<=l;i++) {
*p++=CHAR_SINT(INT_INTOBJ(ELM_PLIST(val,i)));
Obj x = ELM_PLIST(val,i);
if (!IS_INTOBJ(x))
goto again;
*p++=CHAR_SINT(INT_INTOBJ(x));
}
}
else {
Expand All @@ -434,7 +437,6 @@ Obj FuncSTRING_SINTLIST (

}

CHANGED_BAG(n);
return n;
}

Expand Down Expand Up @@ -464,12 +466,10 @@ Obj FuncREVNEG_STRING (
q=CHARS_STRING(n);
j=l-1;
for (i=1;i<=l;i++) {
/* *q++=CHAR_SINT(-SINT_CHAR(p[j])); */
*q++=-p[j];
j--;
}

CHANGED_BAG(n);
return n;
}

Expand Down Expand Up @@ -1084,8 +1084,6 @@ void AssString (

/* now perform the assignment and return the assigned value */
SET_ELM_STRING( list, pos, val );
/* CHARS_STRING(list)[pos-1] = CHAR_VALUE(val); */
CHANGED_BAG( list );
}
}

Expand Down Expand Up @@ -1377,7 +1375,6 @@ void ConvString (
ResizeBag( string, SIZEBAG_STRINGLEN(lenString) );
/* copy data area from tmp */
memcpy(ADDR_OBJ(string), CONST_ADDR_OBJ(tmp), SIZE_OBJ(tmp));
CHANGED_BAG(string);
}


Expand Down Expand Up @@ -1431,31 +1428,13 @@ Obj MakeString2(const Char *cstr1, const Char *cstr2)
return result;
}

Obj MakeString3(const Char *cstr1, const Char *cstr2, const Char *cstr3)
{
Obj result;
size_t len1 = strlen(cstr1), len2 = strlen(cstr2), len3 = strlen(cstr3);
result = NEW_STRING(len1 + len2 + len3);
memcpy(CSTR_STRING(result), cstr1, len1);
memcpy(CSTR_STRING(result)+len1, cstr2, len2);
memcpy(CSTR_STRING(result)+len1+len2, cstr3, len3);
return result;
}

Obj MakeImmString2(const Char *cstr1, const Char *cstr2)
{
Obj result = MakeString2(cstr1, cstr2);
MakeImmutableString(result);
return result;
}

Obj MakeImmString3(const Char *cstr1, const Char *cstr2, const Char *cstr3)
{
Obj result = MakeString3(cstr1, cstr2, cstr3);
MakeImmutableString(result);
return result;
}

Obj ConvImmString(Obj str)
{
Obj result;
Expand Down Expand Up @@ -1511,12 +1490,11 @@ Obj FuncCONV_STRING (
Obj string )
{
/* check whether <string> is a string */
if ( ! IS_STRING( string ) ) {
while ( ! IS_STRING( string ) ) {
string = ErrorReturnObj(
"ConvString: <string> must be a string (not a %s)",
(Int)TNAM_OBJ(string), 0L,
"you can replace <string> via 'return <string>;'" );
return FuncCONV_STRING( self, string );
}

/* convert to the string representation */
Expand Down Expand Up @@ -1549,12 +1527,11 @@ Obj FuncCOPY_TO_STRING_REP (
Obj obj )
{
/* check whether <obj> is a string */
if (!IS_STRING(obj)) {
while (!IS_STRING(obj)) {
obj = ErrorReturnObj(
"ConvString: <string> must be a string (not a %s)",
"CopyToStringRep: <string> must be a string (not a %s)",
(Int)TNAM_OBJ(obj), 0L,
"you can replace <string> via 'return <string>;'" );
return FuncCOPY_TO_STRING_REP( self, obj );
}
return CopyToStringRep(obj);
}
Expand Down Expand Up @@ -1645,12 +1622,11 @@ Obj FuncNormalizeWhitespace (
Int i, j, len, white;

/* check whether <string> is a string */
if ( ! IsStringConv( string ) ) {
while ( ! IsStringConv( string ) ) {
string = ErrorReturnObj(
"NormalizeWhitespace: <string> must be a string (not a %s)",
(Int)TNAM_OBJ(string), 0L,
"you can replace <string> via 'return <string>;'" );
return FuncNormalizeWhitespace( self, string );
}

len = GET_LEN_STRING(string);
Expand Down Expand Up @@ -1701,21 +1677,19 @@ Obj FuncREMOVE_CHARACTERS (
UInt1 REMCHARLIST[256] = {0};

/* check whether <string> is a string */
if ( ! IsStringConv( string ) ) {
while ( ! IsStringConv( string ) ) {
string = ErrorReturnObj(
"RemoveCharacters: first argument <string> must be a string (not a %s)",
(Int)TNAM_OBJ(string), 0L,
"you can replace <string> via 'return <string>;'" );
return FuncREMOVE_CHARACTERS( self, string, rem );
}

/* check whether <rem> is a string */
if ( ! IsStringConv( rem ) ) {
while ( ! IsStringConv( rem ) ) {
rem = ErrorReturnObj(
"RemoveCharacters: second argument <rem> must be a string (not a %s)",
(Int)TNAM_OBJ(rem), 0L,
"you can replace <rem> via 'return <rem>;'" );
return FuncREMOVE_CHARACTERS( self, string, rem );
}

/* set REMCHARLIST by setting positions of characters in rem to 1 */
Expand Down Expand Up @@ -1757,32 +1731,28 @@ Obj FuncTranslateString (
Int j, len;

/* check whether <string> is a string */
if ( ! IsStringConv( string ) ) {
while ( ! IsStringConv( string ) ) {
string = ErrorReturnObj(
"RemoveCharacters: first argument <string> must be a string (not a %s)",
"TranslateString: first argument <string> must be a string (not a %s)",
(Int)TNAM_OBJ(string), 0L,
"you can replace <string> via 'return <string>;'" );
return FuncTranslateString( self, string, trans );
}

/* check whether <trans> is a string */
if ( ! IsStringConv( trans ) ) {
trans = ErrorReturnObj(
"RemoveCharacters: second argument <trans> must be a string (not a %s)",
(Int)TNAM_OBJ(trans), 0L,
"you can replace <trans> via 'return <trans>;'" );
return FuncTranslateString( self, string, trans );
}

/* check if string has length at least 256 */
if ( GET_LEN_STRING( trans ) < 256 ) {
trans = ErrorReturnObj(
"RemoveCharacters: second argument <trans> must have length >= 256",
0L, 0L,
"you can replace <trans> via 'return <trans>;'" );
return FuncTranslateString( self, string, trans );
// check whether <trans> is a string of length at least 256
while ( ! IsStringConv( trans ) || GET_LEN_STRING( trans ) < 256 ) {
if ( ! IsStringConv( trans ) ) {
trans = ErrorReturnObj(
"TranslateString: second argument <trans> must be a string (not a %s)",
(Int)TNAM_OBJ(trans), 0L,
"you can replace <trans> via 'return <trans>;'" );
} else if ( GET_LEN_STRING( trans ) < 256 ) {
trans = ErrorReturnObj(
"TranslateString: second argument <trans> must have length >= 256",
0L, 0L,
"you can replace <trans> via 'return <trans>;'" );
}
}

/* now change string in place */
len = GET_LEN_STRING(string);
s = CHARS_STRING(string);
Expand Down Expand Up @@ -1816,30 +1786,27 @@ Obj FuncSplitStringInternal (
UInt1 SPLITSTRINGWSPACE[256] = { 0 };

/* check whether <string> is a string */
if ( ! IsStringConv( string ) ) {
while ( ! IsStringConv( string ) ) {
string = ErrorReturnObj(
"SplitString: first argument <string> must be a string (not a %s)",
(Int)TNAM_OBJ(string), 0L,
"you can replace <string> via 'return <string>;'" );
return FuncSplitStringInternal( self, string, seps, wspace );
}

/* check whether <seps> is a string */
if ( ! IsStringConv( seps ) ) {
while ( ! IsStringConv( seps ) ) {
seps = ErrorReturnObj(
"SplitString: second argument <seps> must be a string (not a %s)",
(Int)TNAM_OBJ(seps), 0L,
"you can replace <seps> via 'return <seps>;'" );
return FuncSplitStringInternal( self, string, seps, wspace );
}

/* check whether <wspace> is a string */
if ( ! IsStringConv( wspace ) ) {
while ( ! IsStringConv( wspace ) ) {
wspace = ErrorReturnObj(
"SplitString: third argument <wspace> must be a string (not a %s)",
(Int)TNAM_OBJ(wspace), 0L,
"you can replace <wspace> via 'return <wspace>;'" );
return FuncSplitStringInternal( self, string, seps, wspace );
}

/* set SPLITSTRINGSEPS by setting positions of characters in rem to 1 */
Expand Down
2 changes: 0 additions & 2 deletions src/stringobj.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,8 @@ static inline Obj MakeImmString(const Char * cstr)


Obj MakeString2(const Char *cstr1, const Char *cstr2);
Obj MakeString3(const Char *cstr1, const Char *cstr2, const Char *cstr3);

Obj MakeImmString2(const Char *cstr1, const Char *cstr2);
Obj MakeImmString3(const Char *cstr1, const Char *cstr2, const Char *cstr3);
Obj ConvImmString(Obj str);


Expand Down
28 changes: 28 additions & 0 deletions tst/testinstall/kernel/streams.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ gap> INPUT_TEXT_FILE(fail);
Error, <filename> must be a string (not a boolean or fail)
gap> IS_END_OF_FILE(fail);
Error, <fid> must be an integer (not a boolean or fail)
gap> IS_END_OF_FILE(-1);
fail
gap> IS_END_OF_FILE(0);
false
gap> IS_END_OF_FILE(254);
fail
gap> OUTPUT_TEXT_FILE(fail, fail);
Error, <filename> must be a string (not a boolean or fail)
gap> OUTPUT_TEXT_FILE("test", fail);
Expand All @@ -74,8 +80,20 @@ Error, <append> must be a boolean (not a boolean or fail)
#
gap> POSITION_FILE(fail);
Error, <fid> must be an integer (not a boolean or fail)
gap> POSITION_FILE(-1);
fail
gap> IsInt(POSITION_FILE(4));
true
gap> POSITION_FILE(254);
fail

#
gap> READ_BYTE_FILE(fail);
Error, <fid> must be an integer (not a boolean or fail)
gap> READ_BYTE_FILE(-1);
fail

#
gap> READ_LINE_FILE(fail);
Error, <fid> must be an integer (not a boolean or fail)
gap> READ_ALL_FILE(fail,fail);
Expand All @@ -86,10 +104,20 @@ gap> SEEK_POSITION_FILE(fail,fail);
Error, <fid> must be an integer (not a boolean or fail)
gap> SEEK_POSITION_FILE(1,fail);
Error, <pos> must be an integer (not a boolean or fail)

#
gap> WRITE_BYTE_FILE(fail,fail);
Error, <fid> must be an integer (not a boolean or fail)
gap> WRITE_BYTE_FILE(1,fail);
Error, <ch> must be an integer (not a boolean or fail)
gap> WRITE_BYTE_FILE(-1,65);
fail
gap> WRITE_BYTE_FILE(0,65);
true
gap> WRITE_BYTE_FILE(254,65);
fail

#
gap> READ_STRING_FILE(fail);
Error, <fid> must be an integer (not a boolean or fail)

Expand Down
Loading