snoopdave opened a new pull request, #213:
URL: https://github.com/apache/roller/pull/213
Bugs found while running a 7.0.0-SNAPSHOT build (Rollarcus deploy
`dcd21bf35`, which contains master `359c4804c`) on rollerweblogger.org with the
new AtomPub server. Each fix has a test; `CHANGES.md` has a 7.0.0 bug-fix
entry. `mvn clean install` passes on JDK 17 (499 tests, 0 failures). Every item
was also checked over HTTP against a local Roller built from this branch
(Jetty, embedded Derby), not the live site.
### 1. Media upload: file extension from the content type
`MediaCollection.fileExtension()` drops parameters (`; charset=UTF-8`) and a
structured-syntax suffix (`+xml`, `+json`), and maps common types to their
usual extension (`image/svg+xml` → `svg`, `image/jpeg` → `jpg`, `text/plain` →
`txt`, `application/ld+json` → `jsonld`). `withExtension()` keeps a Slug's own
extension when it matches the type (`x.svg`, `photo.jpeg`).
- Locally: `Content-Type: image/svg+xml` with `Slug: roller-atompub-model`
creates `roller-atompub-model.svg`; `Slug: x.svg` stays `x.svg`; `text/plain;
charset=UTF-8` gives `notes.txt`.
- **A correctly named `.svg`** is stored as `image/svg+xml` (from the
container's MIME map) and, being an active type, is served as
`application/octet-stream` with `Content-Disposition: attachment;
filename="roller-atompub-model.svg"` and `X-Content-Type-Options: nosniff`, at
both the edit-media URI and `/mediaresource/`. That policy is unchanged.
- Tests: `MediaCollectionTest`.
### 2. Media collection paging links
The feed id and the `next`/`previous` links are built by one helper:
`/{h}/resources/{offset}` for the default directory,
`/{h}/resources/{dir}/{offset}` for a named one. `path` has no trailing slash,
and `getCollection()` reads the last segment as the offset, so both forms
round-trip.
- Locally: 22 files in `diagrams`; `next` is `.../resources/diagrams/20`,
answers 200 with the last 2 files and a `previous` link to `/0`.
- Test:
`RollerAtomProtocolTest.testMediaCollectionNextLinksLeadToTheNextPage` follows
`next` to page 2 for a named and the default directory. The guide already
documents `/{h}/resources/{dir}/{offset}`; `docs/atompub/examples` are
unaffected (`AtomPubDocExamplesTest` passes).
### 3. Characters that XML 1.0 does not allow
`AtomWriter.xml()` drops every character outside `#x9 | #xA | #xD |
[#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]`, including unpaired
surrogates, and every `writeCharacters`/`writeAttribute` call goes through it.
- Locally: a comment posted with two U+0014 is stored raw (the public page
still has them), and `GET /comments` returns a feed that parses, with the
content `beforeafterend`.
- Tests: `AtomWriterTest.testCharactersIllegalInXmlAreDropped` (comment
content with `\u0014`, entry title with `\u0000` and a lone surrogate,
attribute value with `\u001F`; the feed parses back) and
`testXmlFilterKeepsLegalText`.
### 4. Entry editor: line-height menu shows no values
Reproduced on master. Two causes:
1. **Summernote 0.8.12 writes menu icons as `<i
class="note-icon-menu-check"/>`.** HTML has no self-closing `<i>`; jQuery
before 3.5 expanded it to `<i></i>`, jQuery 3.5+ does not, so the browser keeps
the `<i>` open and the label ends up inside it: `<i
class="note-icon-menu-check"> 1.0</i>`. Summernote hides that `<i>`
(`.note-check li a i { visibility: hidden }`), so the label disappears. jQuery
is 3.7.1 on master and was in 6.1.6, so this is not new in 7.0.
2. **`roller.css` puts a Glyphicons chevron after every `.panel-heading a`**
(for the entry editor's collapsible panels). Summernote's Bootstrap 3 toolbar
is a `.panel-heading`, so every menu item got a chevron. That is the stack of
glyphs.
Fix: upgrade Summernote to 0.9.1 (`org.webjars.npm`), which writes `<i
...></i>` and also fixes 0.8.12's line-height check never being set; and limit
the chevron rule to `a[data-toggle="collapse"]`. Checked in the local editor:
labels 1.0–3.0 show without chevrons, choosing 1.5 sets `style="line-height:
1.5;"`, a draft saves and reloads it, paste runs, no console errors.
- Test: `EntryEditorAssetsTest` checks that the Summernote script loaded by
`head.jsp` is on the classpath and does not build `<i .../>` icons, and that
every `.panel-heading a…:after` rule is limited to collapse toggles.
- Rollarcus uses struts2-bootstrap 4.0.1; master uses 4.0.0 with Bootstrap
3.4.1. The cause is not in either.
### 5. Custom-theme weblogs with templates but no renditions (investigation
only)
Not fixed here. Findings:
- The released `500-to-510` script creates `custom_template_rendition` and
copies `weblog_custom_template.template` into it as STANDARD renditions, but
only runs when the database version is below 510.
- During 5.1 development (2011–2014) the same script created an empty table
named `rol_templatecode` and copied nothing; it was renamed
`roller_templatecode` (ROL-2000, 2014-05) and then `custom_template_rendition`
(ROL-2039, 2014-07, first released in 5.1.2).
- A database upgraded with a 5.1 snapshot was already at 510, so the release
copy never ran. Templates that were edited afterwards got renditions; templates
that never were (here alex, carl, otto, testblog) have none. The leftover
`rol_templatecode` table matches this.
- `PageServlet` then finds no renderer and answers 404. A fallback would
have no template text to fall back to for a custom theme, so the data has to be
repaired. Released upgrade paths are not affected, so I did not add repair code
to the installer.
Suggested repair (check first: does `rol_templatecode` have rows for those
template ids? does `weblog_custom_template` still have a `template` column?):
```sql
-- from the snapshot-era table
insert into custom_template_rendition (id, templateid, template,
templatelang, type)
select r.id, r.templateid, r.template, upper(coalesce(r.templatelang,
'velocity')), upper(r.type)
from rol_templatecode r
where not exists (select 1 from custom_template_rendition c where
c.templateid = r.templateid);
```
If neither source has the text, switching those weblogs to a shared theme
(and back to custom, if wanted) recreates the templates.
### 6. Also fixed: edit-media GET answered 404
Found while testing item 1. `getMediaResource()` looked uploads up by
`originalPath`, but `postMedia()` stored only the directory name there, so
`GET` on every edit-media URI answered 404. It now uses `getMediaFileByPath()`
(as PUT and DELETE do) and loads the content; uploads no longer set
`originalPath`, which is for legacy upload URLs.
`JPAMediaFileManagerImpl.getMediaFileByPath()` returns null instead of throwing
for an unknown directory.
- Locally: the SVG's edit-media URI answers 200 with the headers above.
- Test:
`RollerAtomProtocolTest.testUploadedMediaCanBeReadThroughItsEditMediaUri`
(named and default directory).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
