Re: [yocto] [error-report-web][PATCH V2] Add local.conf and auto.conf into error details

["Changqing Li" <changqing.li@windriver.com>]

correct the mail list to yocto@lists.yoctoproject.org

On 12/11/19 1:45 PM, Changqing Li wrote:
>
> On 11/13/19 6:36 PM, Paul Eggleton wrote:
>> Hi Changqing,
>>
>> Some comments below.
>>
>> On Tuesday, 12 November 2019 9:32:53 PM NZDT 
>> changqing.li@windriver.com wrote:
>>> From: Changqing Li <changqing.li@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@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
> OK, thanks
>>
>>> --- 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.
> OK,  I will fix this.
>>
>>
>>> +        {% 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?
>
> This is for resolve a problem when there is angle brackets in 
> local.conf/auto.conf.
>
> I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: 
> replace angle brackets with &lt; and &gt;]
>
> when we have below content in local.conf or auto.conf:
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> send-error-report will fail with "HTTP Error 500: OK"
>
> error-report-web do rudimentary check on all fields that are
> passed to the graphs page to avoid any XSS happening, if contains
> '<', the server will return error(Invalid characters in json).
> fixed by use escape of <> to replace it.
>
> NOTE: with this change, error-report-web need to add filter 'safe'
> for the string wanted to display to avoid further HTML escaping
> prior to output. Below is how the content displayed on webpage:
> with the filter 'safe':
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
> without the filter 'safe':
> BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"
>
> Do you have good idea to resolve this? Thanks.
>
>>
>> Cheers
>> Paul
>>
>