Skip to content

Commit

Permalink
Fix #344, Consistent directory entry size limit
Browse files Browse the repository at this point in the history
Changes FileName in os_dirent_t from OS_MAX_PATH_LEN to OS_MAX_FILE_NAME,
and moves OS_check_name_length into OS_TranslatePath so it is
consistantly applied everywhere. Also fixes the length checks in
OS_check_name_length to account for terminating null.

Unit tests updated to match new directory name limit.  Coverage
tests updated to match simplified logic.
  • Loading branch information
skliper committed Apr 28, 2020
1 parent 41862fd commit 3fdd398
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 138 deletions.
2 changes: 1 addition & 1 deletion src/bsp/mcp750-vxworks/config/osconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
#define OS_MAX_API_NAME 20

/*
** The maximum length for a file name
** The maximum length (including null terminator) for a file name
*/
#define OS_MAX_FILE_NAME 20

Expand Down
2 changes: 1 addition & 1 deletion src/bsp/pc-linux/config/osconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
#define OS_MAX_API_NAME 20

/*
** The maximum length for a file name
** The maximum length (including null terminator) for a file name
*/
#define OS_MAX_FILE_NAME 20

Expand Down
2 changes: 1 addition & 1 deletion src/bsp/pc-rtems/config/osconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
#define OS_MAX_API_NAME 20

/*
** The maximum length for a file name
** The maximum length (including null terminator) for a file name
*/
#define OS_MAX_FILE_NAME 20

Expand Down
2 changes: 1 addition & 1 deletion src/os/inc/osapi-os-filesys.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ enum
/** @brief Directory entry */
typedef struct
{
char FileName[OS_MAX_PATH_LEN];
char FileName[OS_MAX_FILE_NAME];
} os_dirent_t;

#ifndef OSAL_OMIT_DEPRECATED
Expand Down
4 changes: 2 additions & 2 deletions src/os/portable/os-impl-posix-dirs.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ int32 OS_DirRead_Impl(uint32 local_id, os_dirent_t *dirent)
return OS_ERROR;
}

strncpy(dirent->FileName, de->d_name, OS_MAX_PATH_LEN - 1);
dirent->FileName[OS_MAX_PATH_LEN - 1] = 0;
strncpy(dirent->FileName, de->d_name, sizeof(dirent->FileName) - 1);
dirent->FileName[sizeof(dirent->FileName) - 1] = 0;

return OS_SUCCESS;
} /* end OS_DirRead_Impl */
Expand Down
65 changes: 6 additions & 59 deletions src/os/shared/osapi-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,41 +53,7 @@ enum
};

OS_stream_internal_record_t OS_stream_table[OS_MAX_NUM_OPEN_FILES];


/*----------------------------------------------------------------
*
* Function: OS_check_name_length
*
* Purpose: Local helper routine, not part of OSAL API.
* Validates that the path length is within spec and
* contains at least one directory separator (/) char.
*
*-----------------------------------------------------------------*/
static int32 OS_check_name_length(const char *path)
{
char* name_ptr;

if (path == NULL)
return OS_INVALID_POINTER;

if (strlen(path) > OS_MAX_PATH_LEN)
return OS_FS_ERR_PATH_TOO_LONG;

/* checks to see if there is a '/' somewhere in the path */
name_ptr = strrchr(path, '/');
if (name_ptr == NULL)
return OS_ERROR;

/* strrchr returns a pointer to the last '/' char, so we advance one char */
name_ptr = name_ptr + 1;

if( strlen(name_ptr) > OS_MAX_FILE_NAME)
return OS_FS_ERR_NAME_TOO_LONG;

return OS_SUCCESS;

} /* end OS_check_name_length */

/****************************************************************************************
FILE API
Expand Down Expand Up @@ -126,16 +92,9 @@ static int32 OS_OpenCreate(uint32 *filedes, const char *path, int32 flags, int32
char local_path[OS_MAX_LOCAL_PATH_LEN];

/*
** check if the name of the file is too long
*/
return_code = OS_check_name_length(path);
if (return_code == OS_SUCCESS)
{
/*
** Translate the path
*/
return_code = OS_TranslatePath(path, (char *)local_path);
}
* Translate the path
*/
return_code = OS_TranslatePath(path, (char *)local_path);

if (return_code == OS_SUCCESS)
{
Expand Down Expand Up @@ -455,14 +414,10 @@ int32 OS_remove (const char *path)
int32 return_code;
char local_path[OS_MAX_LOCAL_PATH_LEN];

return_code = OS_check_name_length(path);
return_code = OS_TranslatePath(path, local_path);
if (return_code == OS_SUCCESS)
{
return_code = OS_TranslatePath(path, local_path);
if (return_code == OS_SUCCESS)
{
return_code = OS_FileRemove_Impl(local_path);
}
return_code = OS_FileRemove_Impl(local_path);
}

return return_code;
Expand All @@ -485,15 +440,7 @@ int32 OS_rename (const char *old, const char *new)
char old_path[OS_MAX_LOCAL_PATH_LEN];
char new_path[OS_MAX_LOCAL_PATH_LEN];

return_code = OS_check_name_length(old);
if (return_code == OS_SUCCESS)
{
return_code = OS_check_name_length(new);
}
if (return_code == OS_SUCCESS)
{
return_code = OS_TranslatePath(old, old_path);
}
return_code = OS_TranslatePath(old, old_path);
if (return_code == OS_SUCCESS)
{
return_code = OS_TranslatePath(new, new_path);
Expand Down
49 changes: 42 additions & 7 deletions src/os/shared/osapi-filesys.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,40 @@ extern const OS_VolumeInfo_t OS_VolumeTable[];
#define OS_COMPAT_VOLTAB_SIZE 0
#endif


/*----------------------------------------------------------------
*
* Function: OS_check_name_length
*
* Purpose: Local helper routine, not part of OSAL API.
* Validates that the path length is within spec and
* contains at least one directory separator (/) char.
*
*-----------------------------------------------------------------*/
static int32 OS_check_name_length(const char *path)
{
char* name_ptr;

/* NULL check is done in calling function */

if (strlen(path) >= OS_MAX_PATH_LEN)
return OS_FS_ERR_PATH_TOO_LONG;

/* checks to see if there is a '/' somewhere in the path */
name_ptr = strrchr(path, '/');
if (name_ptr == NULL)
return OS_FS_ERR_PATH_INVALID;

/* strrchr returns a pointer to the last '/' char, so we advance one char */
name_ptr = name_ptr + 1;

if( strlen(name_ptr) >= OS_MAX_FILE_NAME)
return OS_FS_ERR_NAME_TOO_LONG;

return OS_SUCCESS;

} /* end OS_check_name_length */


/*----------------------------------------------------------------
*
Expand Down Expand Up @@ -1061,18 +1095,19 @@ int32 OS_TranslatePath(const char *VirtualPath, char *LocalPath)
return OS_INVALID_POINTER;
}

SysMountPointLen = 0;
VirtPathLen = strlen(VirtualPath);
VirtPathBegin = VirtPathLen;

/*
** Check to see if the path is too long
** Check length
*/
if (VirtPathLen >= OS_MAX_PATH_LEN)
return_code = OS_check_name_length(VirtualPath);
if (return_code != OS_SUCCESS)
{
return OS_FS_ERR_PATH_TOO_LONG;
return return_code;
}

SysMountPointLen = 0;
VirtPathLen = strlen(VirtualPath);
VirtPathBegin = VirtPathLen;

/*
** All valid Virtual paths must start with a '/' character
*/
Expand Down
4 changes: 2 additions & 2 deletions src/os/vxworks/osfileapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ int32 OS_DirRead_Impl(uint32 local_id, os_dirent_t *dirent)
return OS_ERROR;
}

strncpy(dirent->FileName, de->d_name, OS_MAX_PATH_LEN - 1);
dirent->FileName[OS_MAX_PATH_LEN - 1] = 0;
strncpy(dirent->FileName, de->d_name, sizeof(dirent->FileName) - 1);
dirent->FileName[sizeof(dirent->FileName) - 1] = 0;

return OS_SUCCESS;
} /* end OS_DirRead_Impl */
Expand Down
85 changes: 49 additions & 36 deletions src/tests/file-api-test/file-api-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,55 +95,65 @@ void TestCreatRemove(void)
char maxfilename[OS_MAX_PATH_LEN];
char longfilename [OS_MAX_PATH_LEN + 10];
int status;
int fd;
int i;

strncpy(filename,"/drive0/test11chars", sizeof(filename) - 1);
filename[sizeof(filename) - 1] = 0;
/* Short file name */
strncpy(filename,"/drive0/a", sizeof(filename));

/* make the two really long filenames */

/* 1 '/' and 40 'm' */
strncpy(maxfilename,"/drive0/mmmmmmmmmmmmmmmmmmmm", sizeof(maxfilename) - 1);
maxfilename[sizeof(maxfilename) - 1] = 0;
/* Create max file name (OS_MAX_FILE_NAME including terminating null) */
strncpy(maxfilename,"/drive0/", sizeof(maxfilename));
for (i = strlen(maxfilename); i < (OS_MAX_FILE_NAME - 1); i++)
{
maxfilename[i] = 'm';
}

/* 1 '/' and 41 'l' */
strncpy(longfilename,"/drive0/lllllllllllllllllllllllllllllllllllllllll", sizeof(longfilename) - 1);
longfilename[sizeof(longfilename) - 1] = 0;
/* Create longer than max file name */
strncpy(longfilename,"/drive0/", sizeof(longfilename));
for (i = strlen(longfilename); i < (sizeof(longfilename) - 1); i++)
{
longfilename[i] = 'm';
}

/* create a file of reasonable length (but over 8 chars) */
status = OS_creat(filename,OS_READ_WRITE);
UtAssert_True(status >= OS_SUCCESS, "status after creat = %d",(int)status);
/* create a file with short name */
fd = OS_creat(filename,OS_READ_WRITE);
UtAssert_True(fd >= 0, "fd after creat short name length file = %d",(int)fd);

/* close the first file */
status = OS_close(status);
UtAssert_True(status == OS_SUCCESS, "status after close = %d",(int)status);
status = OS_close(fd);
UtAssert_True(status == OS_SUCCESS, "status after close short name length file = %d",(int)status);

/* create a file of OS_max_file_name size */
status = OS_creat(maxfilename,OS_READ_WRITE);
UtAssert_True(status >= OS_SUCCESS, "status after creat = %d",(int)status);
/* create a file with max name size */
fd = OS_creat(maxfilename,OS_READ_WRITE);
UtAssert_True(fd >= 0, "fd after creat max name length file = %d",(int)fd);

/* close the second file */
status = OS_close(status);
UtAssert_True(status == OS_SUCCESS, "status after close = %d",(int)status);
status = OS_close(fd);
UtAssert_True(status == OS_SUCCESS, "status after close max name length file = %d",(int)status);

/* try removing the file from the drive */
/* remove the file from the drive */
status = OS_remove(filename);
UtAssert_True(status == OS_SUCCESS, "status after remove = %d",(int)status);
UtAssert_True(status == OS_SUCCESS, "status after short name length file remove = %d",(int)status);

/* try removing the file from the drive */
/* remove the file from the drive */
status = OS_remove(maxfilename);
UtAssert_True(status == OS_SUCCESS, "status after remove = %d",(int)status);
UtAssert_True(status == OS_SUCCESS, "status after remove max name length file = %d",(int)status);

/* try removing the file from the drive. Should Fail */
/* try creating with file name too big, should fail */
status = OS_creat(longfilename,OS_READ_WRITE);
UtAssert_True(status < OS_SUCCESS, "status after create file name too long = %d",(int)status);

/* try removing with file name too big. Should Fail */
status = OS_remove(longfilename);
UtAssert_True(status < OS_SUCCESS, "status after remove = %d",(int)status);
UtAssert_True(status < OS_SUCCESS, "status after remove file name too long = %d",(int)status);

/* try removing the file from the drive. Should Fail */
status = OS_remove("/drive0/FileNotOnDrive");
UtAssert_True(status < OS_SUCCESS, "status after remove = %d",(int)status);
/* try removing file that no longer exists. Should Fail */
status = OS_remove(filename);
UtAssert_True(status < OS_SUCCESS, "status after remove file doesn't exist = %d",(int)status);

/* Similar to previous but with a bad mount point (gives different error code) */
status = OS_remove("/FileNotOnDrive");
UtAssert_True(status == OS_FS_ERR_PATH_INVALID, "status after remove = %d",(int)status);
status = OS_remove("/x");
UtAssert_True(status == OS_FS_ERR_PATH_INVALID, "status after remove bad mount = %d",(int)status);
}

/*---------------------------------------------------------------------------------------
Expand Down Expand Up @@ -781,13 +791,16 @@ void TestStat(void)

char filename1[OS_MAX_PATH_LEN];
char dir1 [OS_MAX_PATH_LEN];
char dir1slash[OS_MAX_PATH_LEN];
char buffer1 [OS_MAX_PATH_LEN];
os_fstat_t StatBuff;
int32 fd1;
int size;

strcpy(dir1,"/drive0/ThisIsALongDirectoryName");
strcpy(filename1,"/drive0/ThisIsALongDirectoryName/MyFile1");
strcpy(dir1,"/drive0/DirectoryName");
strcpy(dir1slash, dir1);
strcat(dir1slash, "/");
strcpy(filename1,"/drive0/DirectoryName/MyFile1");
strcpy(buffer1,"111111111111");

/* make the directory */
Expand All @@ -812,13 +825,13 @@ void TestStat(void)
/*
** Make the stat calls
*/
status = OS_stat( "/drive0/ThisIsALongDirectoryName/",&StatBuff);
status = OS_stat(dir1slash,&StatBuff);
UtAssert_True(status == OS_SUCCESS, "status after stat 1 = %d",(int)status);

status = OS_stat( "/drive0/ThisIsALongDirectoryName",&StatBuff);
status = OS_stat(dir1,&StatBuff);
UtAssert_True(status == OS_SUCCESS, "status after stat 2 = %d",(int)status);

status = OS_stat( "/drive0/ThisIsALongDirectoryName/MyFile1",&StatBuff);
status = OS_stat(filename1,&StatBuff);
UtAssert_True(status == OS_SUCCESS, "status after stat 3 = %d",(int)status);

/* Clean up */
Expand Down
Loading

0 comments on commit 3fdd398

Please sign in to comment.