Skip to content

Make exception messages and error output binary safe #413

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

Closed
wants to merge 5 commits into from
Closed

Make exception messages and error output binary safe #413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9b1069f
Ensure the exception string is constructed in a binary-safe manner.
LawnGnome Aug 19, 2013
e7ece29
Update the default error handler to be binary-safe.
LawnGnome Aug 19, 2013
38b70e5
Fix CLI error output to stderr.
LawnGnome Aug 19, 2013
7684d20
Add a test for binary-safe exception messages and error output.
LawnGnome Aug 19, 2013
269e801
Remove duplicate line.
LawnGnome Aug 19, 2013
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
Binary file added Zend/tests/exception_011.phpt
View file
Open in desktop
Binary file not shown.
34 changes: 27 additions & 7 deletions Zend/zend_exceptions.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -617,10 +617,26 @@ ZEND_METHOD(exception, __toString)
}

if (Z_STRLEN(message) > 0) {
len = zend_spprintf(&str, 0, "exception '%s' with message '%s' in %s:%ld\nStack trace:\n%s%s%s",
Z_OBJCE_P(exception)->name, Z_STRVAL(message), Z_STRVAL(file), Z_LVAL(line),
(trace && Z_STRLEN_P(trace)) ? Z_STRVAL_P(trace) : "#0 {main}\n",
len ? "\n\nNext " : "", prev_str);
char *prefix, *suffix;
int prefix_len, suffix_len;

prefix_len = zend_spprintf(&prefix, 0, "exception '%s' with message '",
Z_OBJCE_P(exception)->name);

suffix_len = zend_spprintf(&suffix, 0, "' in %s:%ld\nStack trace:\n%s%s%s",
Z_STRVAL(file), Z_LVAL(line),
(trace && Z_STRLEN_P(trace)) ? Z_STRVAL_P(trace) : "#0 {main}\n",
len ? "\n\nNext " : "", prev_str);

len = prefix_len + Z_STRLEN(message) + suffix_len;
str = emalloc(len + 1);
memcpy(str, prefix, prefix_len);
memcpy(str + prefix_len, Z_STRVAL(message), Z_STRLEN(message));
memcpy(str + prefix_len + Z_STRLEN(message), suffix, suffix_len);
str[len] = '\0';

efree(prefix);
efree(suffix);
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't you use %Z here too?

  len = zend_spprintf(&str, 0, "exception '%s' with message '%Z' in %s:%ld\nStack trace:\n%s%s%s",
            Z_OBJCE_P(exception)->name, &message, Z_STRVAL(file), Z_LVAL(line),
            (trace && Z_STRLEN_P(trace)) ? Z_STRVAL_P(trace) : "#0 {main}\n",
            len ? "\n\nNext " : "", prev_str);

} else {
len = zend_spprintf(&str, 0, "exception '%s' in %s:%ld\nStack trace:\n%s%s%s",
Z_OBJCE_P(exception)->name, Z_STRVAL(file), Z_LVAL(line),
Expand All @@ -643,7 +659,7 @@ ZEND_METHOD(exception, __toString)

/* We store the result in the private property string so we can access
* the result in uncaught exception handlers without memleaks. */
zend_update_property_string(default_exception_ce, getThis(), "string", sizeof("string")-1, str TSRMLS_CC);
zend_update_property_stringl(default_exception_ce, getThis(), "string", sizeof("string")-1, str, len TSRMLS_CC);

RETURN_STRINGL(str, len, 0);
}
Expand Down Expand Up @@ -807,7 +823,11 @@ ZEND_API void zend_exception_error(zval *exception, int severity TSRMLS_DC) /* {
if (Z_TYPE_P(str) != IS_STRING) {
zend_error(E_WARNING, "%s::__toString() must return a string", ce_exception->name);
} else {
zend_update_property_string(default_exception_ce, exception, "string", sizeof("string")-1, EG(exception) ? ce_exception->name : Z_STRVAL_P(str) TSRMLS_CC);
if (EG(exception)) {
zend_update_property_string(default_exception_ce, exception, "string", sizeof("string")-1, ce_exception->name TSRMLS_CC);
} else {
zend_update_property_stringl(default_exception_ce, exception, "string", sizeof("string")-1, Z_STRVAL_P(str), Z_STRLEN_P(str) TSRMLS_CC);
}
}
}
zval_ptr_dtor(&str);
Expand Down Expand Up @@ -836,7 +856,7 @@ ZEND_API void zend_exception_error(zval *exception, int severity TSRMLS_DC) /* {
convert_to_string(file);
convert_to_long(line);

zend_error_va(severity, (Z_STRLEN_P(file) > 0) ? Z_STRVAL_P(file) : NULL, Z_LVAL_P(line), "Uncaught %s\n thrown", Z_STRVAL_P(str));
zend_error_va(severity, (Z_STRLEN_P(file) > 0) ? Z_STRVAL_P(file) : NULL, Z_LVAL_P(line), "Uncaught %Z\n thrown", str);
} else {
zend_error(severity, "Uncaught exception '%s'", ce_exception->name);
}
Expand Down
20 changes: 15 additions & 5 deletions main/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1086,24 +1086,34 @@ static void php_error_cb(int type, const char *error_filename, const uint error_
if (type == E_ERROR || type == E_PARSE) {
size_t len;
char *buf = php_escape_html_entities(buffer, buffer_len, &len, 0, ENT_COMPAT, NULL TSRMLS_CC);
php_printf("%s<br />\n<b>%s</b>: %s in <b>%s</b> on line <b>%d</b><br />\n%s", STR_PRINT(prepend_string), error_type_str, buf, error_filename, error_lineno, STR_PRINT(append_string));
php_printf("%s<br />\n<b>%s</b>: ", STR_PRINT(prepend_string), error_type_str);
PHPWRITE(buf, len);
php_printf(" in <b>%s</b> on line <b>%d</b><br />\n%s", error_filename, error_lineno, STR_PRINT(append_string));
efree(buf);
} else {
php_printf("%s<br />\n<b>%s</b>: %s in <b>%s</b> on line <b>%d</b><br />\n%s", STR_PRINT(prepend_string), error_type_str, buffer, error_filename, error_lineno, STR_PRINT(append_string));
php_printf("%s<br />\n<b>%s</b>: ", STR_PRINT(prepend_string), error_type_str);
PHPWRITE(buffer, buffer_len);
php_printf(" in <b>%s</b> on line <b>%d</b><br />\n%s", error_filename, error_lineno, STR_PRINT(append_string));
}
} else {
/* Write CLI/CGI errors to stderr if display_errors = "stderr" */
if ((!strcmp(sapi_module.name, "cli") || !strcmp(sapi_module.name, "cgi")) &&
PG(display_errors) == PHP_DISPLAY_ERRORS_STDERR
) {
#ifdef PHP_WIN32
fprintf(stderr, "%s: %s in %s on line %d\n", error_type_str, buffer, error_filename, error_lineno);
fprintf(stderr, "%s: ", error_type_str);
fwrite(buffer, buffer_len, 1, stderr);
fprintf(stderr, " in %s on line %d\n", error_filename, error_lineno);
fflush(stderr);
#else
fprintf(stderr, "%s: %s in %s on line %d\n", error_type_str, buffer, error_filename, error_lineno);
fprintf(stderr, "%s: ", error_type_str);
fwrite(buffer, buffer_len, 1, stderr);
fprintf(stderr, " in %s on line %d\n", error_filename, error_lineno);
#endif
} else {
php_printf("%s\n%s: %s in %s on line %d\n%s", STR_PRINT(prepend_string), error_type_str, buffer, error_filename, error_lineno, STR_PRINT(append_string));
php_printf("%s\n%s: ", STR_PRINT(prepend_string), error_type_str);
PHPWRITE(buffer, buffer_len);
php_printf(" in %s on line %d\n%s", error_filename, error_lineno, STR_PRINT(append_string));
}
}
}
Expand Down