| Message ID | 20190806122049.11455-1-mpe@ellerman.id.au |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | models: Add commit_url_format to Project | expand |
On 6/8/19 10:20 pm, Michael Ellerman wrote: > Add a new field to Project, commit_url_format, which specifies a > format string that can be used to generate a link to a particular > commit for a project. > > This is used in the display of a patch, to render the patch's commit > as a clickable link back to the commit on the SCM website. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Argh, I've actually got a series of my own pending to do exactly this, just had to tidy up the documentation before sending it :) I'll take a look at this and compare later today. > --- > > Passes tox tests. > Not entirely sure about the schema changes, I just cribbed from the > existing fields. > I think the use of mark_safe() is correct, but would appreciate some > review on that. > --- > docs/api/schemas/latest/patchwork.yaml | 11 ++++++++++ > docs/api/schemas/patchwork.j2 | 11 ++++++++++ > docs/api/schemas/v1.0/patchwork.yaml | 11 ++++++++++ > docs/api/schemas/v1.1/patchwork.yaml | 11 ++++++++++ > patchwork/api/embedded.py | 3 ++- > patchwork/api/project.py | 4 ++-- > .../0034_project_commit_url_format.py | 20 +++++++++++++++++++ > patchwork/models.py | 9 +++++++++ > patchwork/templates/patchwork/submission.html | 2 +- > patchwork/templatetags/patch.py | 12 +++++++++++ > 10 files changed, 90 insertions(+), 4 deletions(-) > create mode 100644 patchwork/migrations/0034_project_commit_url_format.py > > diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml > index 724b05e..c9e6c4f 100644 > --- a/docs/api/schemas/latest/patchwork.yaml > +++ b/docs/api/schemas/latest/patchwork.yaml > @@ -1846,6 +1846,9 @@ openapi: '3.0.0' > type: string > format: uri > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > maintainers: > type: array > items: > @@ -2162,6 +2165,10 @@ openapi: '3.0.0' > format: uri > readOnly: true > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > SeriesEmbedded: > type: object > properties: > @@ -2301,6 +2308,10 @@ openapi: '3.0.0' > type: string > format: uri > readOnly: true > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > ErrorUserUpdate: > type: object > properties: > diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 > index 5e2f5e4..c0676f2 100644 > --- a/docs/api/schemas/patchwork.j2 > +++ b/docs/api/schemas/patchwork.j2 > @@ -1861,6 +1861,9 @@ openapi: '3.0.0' > type: string > format: uri > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > maintainers: > type: array > items: > @@ -2185,6 +2188,10 @@ openapi: '3.0.0' > format: uri > readOnly: true > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > SeriesEmbedded: > type: object > properties: > @@ -2326,6 +2333,10 @@ openapi: '3.0.0' > type: string > format: uri > readOnly: true > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > ErrorUserUpdate: > type: object > properties: > diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml > index 02f3a15..370dffe 100644 > --- a/docs/api/schemas/v1.0/patchwork.yaml > +++ b/docs/api/schemas/v1.0/patchwork.yaml > @@ -1811,6 +1811,9 @@ openapi: '3.0.0' > type: string > format: uri > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > maintainers: > type: array > items: > @@ -2101,6 +2104,10 @@ openapi: '3.0.0' > format: uri > readOnly: true > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > SeriesEmbedded: > type: object > properties: > @@ -2235,6 +2242,10 @@ openapi: '3.0.0' > type: string > format: uri > readOnly: true > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > ErrorUserUpdate: > type: object > properties: > diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml > index 0c086ed..778d10f 100644 > --- a/docs/api/schemas/v1.1/patchwork.yaml > +++ b/docs/api/schemas/v1.1/patchwork.yaml > @@ -1846,6 +1846,9 @@ openapi: '3.0.0' > type: string > format: uri > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > maintainers: > type: array > items: > @@ -2162,6 +2165,10 @@ openapi: '3.0.0' > format: uri > readOnly: true > maxLength: 2000 > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > SeriesEmbedded: > type: object > properties: > @@ -2301,6 +2308,10 @@ openapi: '3.0.0' > type: string > format: uri > readOnly: true > + commit_url_format: > + title: Web SCM URL format for a particular commit > + type: string > + readOnly: true > ErrorUserUpdate: > type: object > properties: > diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py > index 60fb9a4..305594a 100644 > --- a/patchwork/api/embedded.py > +++ b/patchwork/api/embedded.py > @@ -158,7 +158,8 @@ from patchwork import models > class Meta: > model = models.Project > fields = ('id', 'url', 'name', 'link_name', 'list_id', > - 'list_email', 'web_url', 'scm_url', 'webscm_url') > + 'list_email', 'web_url', 'scm_url', 'webscm_url', > + 'commit_url_format') > read_only_fields = fields > extra_kwargs = { > 'url': {'view_name': 'api-project-detail'}, > diff --git a/patchwork/api/project.py b/patchwork/api/project.py > index d7bb1f2..408e9db 100644 > --- a/patchwork/api/project.py > +++ b/patchwork/api/project.py > @@ -26,7 +26,7 @@ from patchwork.models import Project > model = Project > fields = ('id', 'url', 'name', 'link_name', 'list_id', 'list_email', > 'web_url', 'scm_url', 'webscm_url', 'maintainers', > - 'subject_match') > + 'subject_match', 'commit_url_format') > read_only_fields = ('name', 'link_name', 'list_id', 'list_email', > 'maintainers', 'subject_match') > versioned_fields = { > @@ -68,7 +68,7 @@ from patchwork.models import Project > """List projects.""" > > search_fields = ('link_name', 'list_id', 'list_email', 'web_url', > - 'scm_url', 'webscm_url') > + 'scm_url', 'webscm_url', 'commit_url_format') > ordering_fields = ('id', 'name', 'link_name', 'list_id') > ordering = 'id' > > diff --git a/patchwork/migrations/0034_project_commit_url_format.py b/patchwork/migrations/0034_project_commit_url_format.py > new file mode 100644 > index 0000000..4670674 > --- /dev/null > +++ b/patchwork/migrations/0034_project_commit_url_format.py > @@ -0,0 +1,20 @@ > +# -*- coding: utf-8 -*- > +# Generated by Django 1.11.22 on 2019-08-06 21:16 > +from __future__ import unicode_literals > + > +from django.db import migrations, models > + > + > +class Migration(migrations.Migration): > + > + dependencies = [ > + ('patchwork', '0033_remove_patch_series_model'), > + ] > + > + operations = [ > + migrations.AddField( > + model_name='project', > + name='commit_url_format', > + field=models.CharField(blank=True, help_text=b'SCM web URL for a particular commit. The string will be passed to str.format(), the commit SHA will be named "commit". Example format for cgit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id={commit} or github: https://github.com/torvalds/linux/commit/{commit}', max_length=2000), > + ), > + ] > diff --git a/patchwork/models.py b/patchwork/models.py > index a7eee4d..fd2335e 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -77,6 +77,15 @@ from patchwork.hasher import hash_diff > web_url = models.CharField(max_length=2000, blank=True) > scm_url = models.CharField(max_length=2000, blank=True) > webscm_url = models.CharField(max_length=2000, blank=True) > + commit_url_format = models.CharField( > + max_length=2000, > + blank=True, > + help_text='SCM web URL for a particular commit. The string will be ' > + 'passed to str.format(), the commit SHA will be named "commit". ' > + 'Example format for cgit: https://git.kernel.org/pub/scm/linux/' > + 'kernel/git/torvalds/linux.git/commit/?id={commit} or github: ' > + 'https://github.com/torvalds/linux/commit/{commit}' > + ) > > # configuration options > > diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html > index b1ae542..ea673d1 100644 > --- a/patchwork/templates/patchwork/submission.html > +++ b/patchwork/templates/patchwork/submission.html > @@ -45,7 +45,7 @@ function toggle_div(link_id, headers_id) > {% if submission.commit_ref %} > <tr> > <th>Commit</th> > - <td>{{ submission.commit_ref }}</td> > + <td>{{ submission|patch_commit_display }}</td> > </tr> > {% endif %} > {% if submission.delegate %} > diff --git a/patchwork/templatetags/patch.py b/patchwork/templatetags/patch.py > index 757f873..628725d 100644 > --- a/patchwork/templatetags/patch.py > +++ b/patchwork/templatetags/patch.py > @@ -66,3 +66,15 @@ register = template.Library() > @stringfilter > def msgid(value): > return escape(value.strip('<>')) > + > + > +@register.filter(name='patch_commit_display') > +def patch_commit_display(patch): > + commit = escape(patch.commit_ref) > + if not patch.project.commit_url_format: > + return commit > + > + # NB. This is safe because we escaped commit above and the format string > + # can only be set by an administrator. > + return mark_safe('<a href="%s">%s</a>' % ( > + patch.project.commit_url_format.format(commit=commit), commit)) >
On 7/8/19 9:22 am, Andrew Donnellan wrote: > On 6/8/19 10:20 pm, Michael Ellerman wrote: >> Add a new field to Project, commit_url_format, which specifies a >> format string that can be used to generate a link to a particular >> commit for a project. >> >> This is used in the display of a patch, to render the patch's commit >> as a clickable link back to the commit on the SCM website. >> >> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > > Argh, I've actually got a series of my own pending to do exactly this, > just had to tidy up the documentation before sending it :) > > I'll take a look at this and compare later today. I correct myself, I have patches to add mailing list archive links, which is slightly different! My series includes a minor bump to the API versioning, which per docs/development/releasing.rst is our policy when adding new fields. I'll tidy that up and send it and perhaps you can rebase your API changes on top of that? > >> --- >> >> Passes tox tests. >> Not entirely sure about the schema changes, I just cribbed from the >> existing fields. >> I think the use of mark_safe() is correct, but would appreciate some >> review on that. >> --- >> docs/api/schemas/latest/patchwork.yaml | 11 ++++++++++ >> docs/api/schemas/patchwork.j2 | 11 ++++++++++ >> docs/api/schemas/v1.0/patchwork.yaml | 11 ++++++++++ >> docs/api/schemas/v1.1/patchwork.yaml | 11 ++++++++++ >> patchwork/api/embedded.py | 3 ++- >> patchwork/api/project.py | 4 ++-- >> .../0034_project_commit_url_format.py | 20 +++++++++++++++++++ >> patchwork/models.py | 9 +++++++++ >> patchwork/templates/patchwork/submission.html | 2 +- >> patchwork/templatetags/patch.py | 12 +++++++++++ >> 10 files changed, 90 insertions(+), 4 deletions(-) >> create mode 100644 >> patchwork/migrations/0034_project_commit_url_format.py >> >> diff --git a/docs/api/schemas/latest/patchwork.yaml >> b/docs/api/schemas/latest/patchwork.yaml >> index 724b05e..c9e6c4f 100644 >> --- a/docs/api/schemas/latest/patchwork.yaml >> +++ b/docs/api/schemas/latest/patchwork.yaml >> @@ -1846,6 +1846,9 @@ openapi: '3.0.0' >> type: string >> format: uri >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> maintainers: >> type: array >> items: >> @@ -2162,6 +2165,10 @@ openapi: '3.0.0' >> format: uri >> readOnly: true >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> SeriesEmbedded: >> type: object >> properties: >> @@ -2301,6 +2308,10 @@ openapi: '3.0.0' >> type: string >> format: uri >> readOnly: true >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> ErrorUserUpdate: >> type: object >> properties: >> diff --git a/docs/api/schemas/patchwork.j2 >> b/docs/api/schemas/patchwork.j2 >> index 5e2f5e4..c0676f2 100644 >> --- a/docs/api/schemas/patchwork.j2 >> +++ b/docs/api/schemas/patchwork.j2 >> @@ -1861,6 +1861,9 @@ openapi: '3.0.0' >> type: string >> format: uri >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> maintainers: >> type: array >> items: >> @@ -2185,6 +2188,10 @@ openapi: '3.0.0' >> format: uri >> readOnly: true >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> SeriesEmbedded: >> type: object >> properties: >> @@ -2326,6 +2333,10 @@ openapi: '3.0.0' >> type: string >> format: uri >> readOnly: true >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> ErrorUserUpdate: >> type: object >> properties: >> diff --git a/docs/api/schemas/v1.0/patchwork.yaml >> b/docs/api/schemas/v1.0/patchwork.yaml >> index 02f3a15..370dffe 100644 >> --- a/docs/api/schemas/v1.0/patchwork.yaml >> +++ b/docs/api/schemas/v1.0/patchwork.yaml >> @@ -1811,6 +1811,9 @@ openapi: '3.0.0' >> type: string >> format: uri >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> maintainers: >> type: array >> items: >> @@ -2101,6 +2104,10 @@ openapi: '3.0.0' >> format: uri >> readOnly: true >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> SeriesEmbedded: >> type: object >> properties: >> @@ -2235,6 +2242,10 @@ openapi: '3.0.0' >> type: string >> format: uri >> readOnly: true >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> ErrorUserUpdate: >> type: object >> properties: >> diff --git a/docs/api/schemas/v1.1/patchwork.yaml >> b/docs/api/schemas/v1.1/patchwork.yaml >> index 0c086ed..778d10f 100644 >> --- a/docs/api/schemas/v1.1/patchwork.yaml >> +++ b/docs/api/schemas/v1.1/patchwork.yaml >> @@ -1846,6 +1846,9 @@ openapi: '3.0.0' >> type: string >> format: uri >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> maintainers: >> type: array >> items: >> @@ -2162,6 +2165,10 @@ openapi: '3.0.0' >> format: uri >> readOnly: true >> maxLength: 2000 >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> SeriesEmbedded: >> type: object >> properties: >> @@ -2301,6 +2308,10 @@ openapi: '3.0.0' >> type: string >> format: uri >> readOnly: true >> + commit_url_format: >> + title: Web SCM URL format for a particular commit >> + type: string >> + readOnly: true >> ErrorUserUpdate: >> type: object >> properties: >> diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py >> index 60fb9a4..305594a 100644 >> --- a/patchwork/api/embedded.py >> +++ b/patchwork/api/embedded.py >> @@ -158,7 +158,8 @@ from patchwork import models >> class Meta: >> model = models.Project >> fields = ('id', 'url', 'name', 'link_name', 'list_id', >> - 'list_email', 'web_url', 'scm_url', 'webscm_url') >> + 'list_email', 'web_url', 'scm_url', 'webscm_url', >> + 'commit_url_format') >> read_only_fields = fields >> extra_kwargs = { >> 'url': {'view_name': 'api-project-detail'}, >> diff --git a/patchwork/api/project.py b/patchwork/api/project.py >> index d7bb1f2..408e9db 100644 >> --- a/patchwork/api/project.py >> +++ b/patchwork/api/project.py >> @@ -26,7 +26,7 @@ from patchwork.models import Project >> model = Project >> fields = ('id', 'url', 'name', 'link_name', 'list_id', >> 'list_email', >> 'web_url', 'scm_url', 'webscm_url', 'maintainers', >> - 'subject_match') >> + 'subject_match', 'commit_url_format') >> read_only_fields = ('name', 'link_name', 'list_id', >> 'list_email', >> 'maintainers', 'subject_match') >> versioned_fields = { >> @@ -68,7 +68,7 @@ from patchwork.models import Project >> """List projects.""" >> search_fields = ('link_name', 'list_id', 'list_email', 'web_url', >> - 'scm_url', 'webscm_url') >> + 'scm_url', 'webscm_url', 'commit_url_format') >> ordering_fields = ('id', 'name', 'link_name', 'list_id') >> ordering = 'id' >> diff --git a/patchwork/migrations/0034_project_commit_url_format.py >> b/patchwork/migrations/0034_project_commit_url_format.py >> new file mode 100644 >> index 0000000..4670674 >> --- /dev/null >> +++ b/patchwork/migrations/0034_project_commit_url_format.py >> @@ -0,0 +1,20 @@ >> +# -*- coding: utf-8 -*- >> +# Generated by Django 1.11.22 on 2019-08-06 21:16 >> +from __future__ import unicode_literals >> + >> +from django.db import migrations, models >> + >> + >> +class Migration(migrations.Migration): >> + >> + dependencies = [ >> + ('patchwork', '0033_remove_patch_series_model'), >> + ] >> + >> + operations = [ >> + migrations.AddField( >> + model_name='project', >> + name='commit_url_format', >> + field=models.CharField(blank=True, help_text=b'SCM web >> URL for a particular commit. The string will be passed to >> str.format(), the commit SHA will be named "commit". Example format >> for cgit: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id={commit} >> or github: https://github.com/torvalds/linux/commit/{commit}', >> max_length=2000), >> + ), >> + ] >> diff --git a/patchwork/models.py b/patchwork/models.py >> index a7eee4d..fd2335e 100644 >> --- a/patchwork/models.py >> +++ b/patchwork/models.py >> @@ -77,6 +77,15 @@ from patchwork.hasher import hash_diff >> web_url = models.CharField(max_length=2000, blank=True) >> scm_url = models.CharField(max_length=2000, blank=True) >> webscm_url = models.CharField(max_length=2000, blank=True) >> + commit_url_format = models.CharField( >> + max_length=2000, >> + blank=True, >> + help_text='SCM web URL for a particular commit. The string >> will be ' >> + 'passed to str.format(), the commit SHA will be named >> "commit". ' >> + 'Example format for cgit: https://git.kernel.org/pub/scm/linux/' >> + 'kernel/git/torvalds/linux.git/commit/?id={commit} or github: ' >> + 'https://github.com/torvalds/linux/commit/{commit}' >> + ) >> # configuration options >> diff --git a/patchwork/templates/patchwork/submission.html >> b/patchwork/templates/patchwork/submission.html >> index b1ae542..ea673d1 100644 >> --- a/patchwork/templates/patchwork/submission.html >> +++ b/patchwork/templates/patchwork/submission.html >> @@ -45,7 +45,7 @@ function toggle_div(link_id, headers_id) >> {% if submission.commit_ref %} >> <tr> >> <th>Commit</th> >> - <td>{{ submission.commit_ref }}</td> >> + <td>{{ submission|patch_commit_display }}</td> >> </tr> >> {% endif %} >> {% if submission.delegate %} >> diff --git a/patchwork/templatetags/patch.py >> b/patchwork/templatetags/patch.py >> index 757f873..628725d 100644 >> --- a/patchwork/templatetags/patch.py >> +++ b/patchwork/templatetags/patch.py >> @@ -66,3 +66,15 @@ register = template.Library() >> @stringfilter >> def msgid(value): >> return escape(value.strip('<>')) >> + >> + >> +@register.filter(name='patch_commit_display') >> +def patch_commit_display(patch): >> + commit = escape(patch.commit_ref) >> + if not patch.project.commit_url_format: >> + return commit >> + >> + # NB. This is safe because we escaped commit above and the format >> string >> + # can only be set by an administrator. >> + return mark_safe('<a href="%s">%s</a>' % ( >> + patch.project.commit_url_format.format(commit=commit), commit)) >> >
Andrew Donnellan <ajd@linux.ibm.com> writes: > On 7/8/19 9:22 am, Andrew Donnellan wrote: >> On 6/8/19 10:20 pm, Michael Ellerman wrote: >>> Add a new field to Project, commit_url_format, which specifies a >>> format string that can be used to generate a link to a particular >>> commit for a project. >>> >>> This is used in the display of a patch, to render the patch's commit >>> as a clickable link back to the commit on the SCM website. >>> >>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> >> >> Argh, I've actually got a series of my own pending to do exactly this, >> just had to tidy up the documentation before sending it :) >> >> I'll take a look at this and compare later today. > > I correct myself, I have patches to add mailing list archive links, > which is slightly different! Phew! > My series includes a minor bump to the API versioning, which per > docs/development/releasing.rst is our policy when adding new fields. > I'll tidy that up and send it and perhaps you can rebase your API > changes on top of that? Sure, just let me know. cheers
Michael Ellerman <mpe@ellerman.id.au> writes: > Andrew Donnellan <ajd@linux.ibm.com> writes: >> On 7/8/19 9:22 am, Andrew Donnellan wrote: >>> On 6/8/19 10:20 pm, Michael Ellerman wrote: >>>> Add a new field to Project, commit_url_format, which specifies a >>>> format string that can be used to generate a link to a particular >>>> commit for a project. >>>> >>>> This is used in the display of a patch, to render the patch's commit >>>> as a clickable link back to the commit on the SCM website. >>>> >>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> >>> >>> Argh, I've actually got a series of my own pending to do exactly this, >>> just had to tidy up the documentation before sending it :) >>> >>> I'll take a look at this and compare later today. >> >> I correct myself, I have patches to add mailing list archive links, >> which is slightly different! > > Phew! > >> My series includes a minor bump to the API versioning, which per >> docs/development/releasing.rst is our policy when adding new fields. >> I'll tidy that up and send it and perhaps you can rebase your API >> changes on top of that? > > Sure, just let me know. It looks like you're going to do a v2 anyway to mesh with Andrew's changes - please could you pop in update to the fixtures that demonstrates/exercises this? I've had a look at the mark_safe bit. I don't love it - it allows someone with priv-esc to admin to XSS everyone who visits a patch page. Having said that I'm not entirely sure what the best way to handle it is. Andrew you did a few follow-up patches for our XSS adventures - do you have any thoughts? Regards, Daniel > > cheers > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On 22/8/19 11:55 am, Daniel Axtens wrote: > It looks like you're going to do a v2 anyway to mesh with Andrew's > changes - please could you pop in update to the fixtures that > demonstrates/exercises this? > > I've had a look at the mark_safe bit. I don't love it - it allows > someone with priv-esc to admin to XSS everyone who visits a patch > page. Having said that I'm not entirely sure what the best way to handle > it is. Andrew you did a few follow-up patches for our XSS adventures - > do you have any thoughts? I think you probably want to wrap the patch.project.commit_url_format.format(commit=commit) in an escape.
diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml index 724b05e..c9e6c4f 100644 --- a/docs/api/schemas/latest/patchwork.yaml +++ b/docs/api/schemas/latest/patchwork.yaml @@ -1846,6 +1846,9 @@ openapi: '3.0.0' type: string format: uri maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string maintainers: type: array items: @@ -2162,6 +2165,10 @@ openapi: '3.0.0' format: uri readOnly: true maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true SeriesEmbedded: type: object properties: @@ -2301,6 +2308,10 @@ openapi: '3.0.0' type: string format: uri readOnly: true + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true ErrorUserUpdate: type: object properties: diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 index 5e2f5e4..c0676f2 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -1861,6 +1861,9 @@ openapi: '3.0.0' type: string format: uri maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string maintainers: type: array items: @@ -2185,6 +2188,10 @@ openapi: '3.0.0' format: uri readOnly: true maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true SeriesEmbedded: type: object properties: @@ -2326,6 +2333,10 @@ openapi: '3.0.0' type: string format: uri readOnly: true + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true ErrorUserUpdate: type: object properties: diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml index 02f3a15..370dffe 100644 --- a/docs/api/schemas/v1.0/patchwork.yaml +++ b/docs/api/schemas/v1.0/patchwork.yaml @@ -1811,6 +1811,9 @@ openapi: '3.0.0' type: string format: uri maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string maintainers: type: array items: @@ -2101,6 +2104,10 @@ openapi: '3.0.0' format: uri readOnly: true maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true SeriesEmbedded: type: object properties: @@ -2235,6 +2242,10 @@ openapi: '3.0.0' type: string format: uri readOnly: true + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true ErrorUserUpdate: type: object properties: diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml index 0c086ed..778d10f 100644 --- a/docs/api/schemas/v1.1/patchwork.yaml +++ b/docs/api/schemas/v1.1/patchwork.yaml @@ -1846,6 +1846,9 @@ openapi: '3.0.0' type: string format: uri maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string maintainers: type: array items: @@ -2162,6 +2165,10 @@ openapi: '3.0.0' format: uri readOnly: true maxLength: 2000 + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true SeriesEmbedded: type: object properties: @@ -2301,6 +2308,10 @@ openapi: '3.0.0' type: string format: uri readOnly: true + commit_url_format: + title: Web SCM URL format for a particular commit + type: string + readOnly: true ErrorUserUpdate: type: object properties: diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index 60fb9a4..305594a 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -158,7 +158,8 @@ from patchwork import models class Meta: model = models.Project fields = ('id', 'url', 'name', 'link_name', 'list_id', - 'list_email', 'web_url', 'scm_url', 'webscm_url') + 'list_email', 'web_url', 'scm_url', 'webscm_url', + 'commit_url_format') read_only_fields = fields extra_kwargs = { 'url': {'view_name': 'api-project-detail'}, diff --git a/patchwork/api/project.py b/patchwork/api/project.py index d7bb1f2..408e9db 100644 --- a/patchwork/api/project.py +++ b/patchwork/api/project.py @@ -26,7 +26,7 @@ from patchwork.models import Project model = Project fields = ('id', 'url', 'name', 'link_name', 'list_id', 'list_email', 'web_url', 'scm_url', 'webscm_url', 'maintainers', - 'subject_match') + 'subject_match', 'commit_url_format') read_only_fields = ('name', 'link_name', 'list_id', 'list_email', 'maintainers', 'subject_match') versioned_fields = { @@ -68,7 +68,7 @@ from patchwork.models import Project """List projects.""" search_fields = ('link_name', 'list_id', 'list_email', 'web_url', - 'scm_url', 'webscm_url') + 'scm_url', 'webscm_url', 'commit_url_format') ordering_fields = ('id', 'name', 'link_name', 'list_id') ordering = 'id' diff --git a/patchwork/migrations/0034_project_commit_url_format.py b/patchwork/migrations/0034_project_commit_url_format.py new file mode 100644 index 0000000..4670674 --- /dev/null +++ b/patchwork/migrations/0034_project_commit_url_format.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.22 on 2019-08-06 21:16 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('patchwork', '0033_remove_patch_series_model'), + ] + + operations = [ + migrations.AddField( + model_name='project', + name='commit_url_format', + field=models.CharField(blank=True, help_text=b'SCM web URL for a particular commit. The string will be passed to str.format(), the commit SHA will be named "commit". Example format for cgit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id={commit} or github: https://github.com/torvalds/linux/commit/{commit}', max_length=2000), + ), + ] diff --git a/patchwork/models.py b/patchwork/models.py index a7eee4d..fd2335e 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -77,6 +77,15 @@ from patchwork.hasher import hash_diff web_url = models.CharField(max_length=2000, blank=True) scm_url = models.CharField(max_length=2000, blank=True) webscm_url = models.CharField(max_length=2000, blank=True) + commit_url_format = models.CharField( + max_length=2000, + blank=True, + help_text='SCM web URL for a particular commit. The string will be ' + 'passed to str.format(), the commit SHA will be named "commit". ' + 'Example format for cgit: https://git.kernel.org/pub/scm/linux/' + 'kernel/git/torvalds/linux.git/commit/?id={commit} or github: ' + 'https://github.com/torvalds/linux/commit/{commit}' + ) # configuration options diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html index b1ae542..ea673d1 100644 --- a/patchwork/templates/patchwork/submission.html +++ b/patchwork/templates/patchwork/submission.html @@ -45,7 +45,7 @@ function toggle_div(link_id, headers_id) {% if submission.commit_ref %} <tr> <th>Commit</th> - <td>{{ submission.commit_ref }}</td> + <td>{{ submission|patch_commit_display }}</td> </tr> {% endif %} {% if submission.delegate %} diff --git a/patchwork/templatetags/patch.py b/patchwork/templatetags/patch.py index 757f873..628725d 100644 --- a/patchwork/templatetags/patch.py +++ b/patchwork/templatetags/patch.py @@ -66,3 +66,15 @@ register = template.Library() @stringfilter def msgid(value): return escape(value.strip('<>')) + + +@register.filter(name='patch_commit_display') +def patch_commit_display(patch): + commit = escape(patch.commit_ref) + if not patch.project.commit_url_format: + return commit + + # NB. This is safe because we escaped commit above and the format string + # can only be set by an administrator. + return mark_safe('<a href="%s">%s</a>' % ( + patch.project.commit_url_format.format(commit=commit), commit))
Add a new field to Project, commit_url_format, which specifies a format string that can be used to generate a link to a particular commit for a project. This is used in the display of a patch, to render the patch's commit as a clickable link back to the commit on the SCM website. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- Passes tox tests. Not entirely sure about the schema changes, I just cribbed from the existing fields. I think the use of mark_safe() is correct, but would appreciate some review on that. --- docs/api/schemas/latest/patchwork.yaml | 11 ++++++++++ docs/api/schemas/patchwork.j2 | 11 ++++++++++ docs/api/schemas/v1.0/patchwork.yaml | 11 ++++++++++ docs/api/schemas/v1.1/patchwork.yaml | 11 ++++++++++ patchwork/api/embedded.py | 3 ++- patchwork/api/project.py | 4 ++-- .../0034_project_commit_url_format.py | 20 +++++++++++++++++++ patchwork/models.py | 9 +++++++++ patchwork/templates/patchwork/submission.html | 2 +- patchwork/templatetags/patch.py | 12 +++++++++++ 10 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 patchwork/migrations/0034_project_commit_url_format.py