[yocto] [error-report-web][PATCH V2] Add local.conf and auto.conf into error details
Paul Eggleton
paul.eggleton at linux.intel.com
Wed Nov 13 02:36:11 PST 2019
Hi Changqing,
Some comments below.
On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li at windriver.com wrote:
> From: Changqing Li <changqing.li at windriver.com>
>
> Support to display local.conf and auto.conf on error report web.
> Here is commit in oe-core, which add local.conf/auto.conf into error report
> https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6
>
> This commit is related to YOCTO #13252
>
> Signed-off-by: Changqing Li <changqing.li at windriver.com>
> ---
> Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
> Post/models.py | 2 ++
> Post/parser.py | 2 ++
> Post/test.py | 2 ++
> templates/error-details.html | 10 ++++++++++
> test-data/test-payload.json | 4 +++-
> 6 files changed, 43 insertions(+), 1 deletion(-)
> create mode 100644 Post/0006_auto_20190917_0419.py
>
> diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
> new file mode 100644
> index 0000000..827944e
> --- /dev/null
> +++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf
> --- a/Post/models.py
> +++ b/Post/models.py
> @@ -43,6 +43,8 @@ class Build(models.Model):
> LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
> ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
> default=ErrorType.RECIPE)
> + LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
> + AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:
1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did
However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.
Another thing, instead of default="" you should use blank=True.
> + {% if detail.BUILD.LOCAL_CONF != "" %}
> + <dt></a>Local Conf:</dt>
> + <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
> + {% endif %}
> +
> + {% if detail.BUILD.AUTO_CONF != "" %}
> + <dt></a>Auto Conf:</dt>
> + <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
> + {% endif %}
We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?
Cheers
Paul
--
Paul Eggleton
Intel System Software Products
More information about the yocto
mailing list