Skip to content
Closed
Changes from 1 commit
Commits
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
6 changes: 5 additions & 1 deletion src/attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/

#include <assert.h>
#include <stdio.h>
#include <string.h>
#include "attr.h"
#include "booth.h"
#include "ticket.h"
#include "pacemaker.h"

Expand Down Expand Up @@ -274,8 +277,9 @@ int store_geo_attr(struct ticket_config *tk, const char *name, char *val, int no
if (!notime)
get_time(&a->update_ts);

assert(strnlen(name, BOOTH_NAME_LEN) < BOOTH_NAME_LEN);

Choose a reason for hiding this comment

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

I'd rather have booth continue to run, even with a truncated attribute name. iirc, this was not unpremeditated.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I understand that it might be too easy (accidentally or not) to push booth layer out of the function with a single crafted geo attribute in CIB/crm_ticket one-liner. So does turning that into warning sounds better? Then, we should likely warn also in case of trying to set/delete such a troublesome (too long in either name or value) attribute being provided by the user to geostore command.

Choose a reason for hiding this comment

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

On Tue, Apr 19, 2016 at 06:26:49AM -0700, Jan Pokorný wrote:

@@ -274,8 +277,9 @@ int store_geo_attr(struct ticket_config *tk, const char *name, char *val, int no
if (!notime)
get_time(&a->update_ts);

  • assert(strnlen(name, BOOTH_NAME_LEN) < BOOTH_NAME_LEN);

OK, I understand that it might be too easy (accidentally or not) to push booth layer out of the function with a single crafted geo attribute in CIB/crm_ticket one-liner. So does turning that into warning sounds better? Then, we should likely warn also in case of trying to set/delete such a troublesome (too long in either name or value) attribute being provided by the user to geostore command.

Yes, warning seems to be better.

g_hash_table_insert(tk->attr,
g_strndup(name, BOOTH_NAME_LEN), a);
g_strndup(name, BOOTH_NAME_LEN-1), a);

return 0;
}
Expand Down