ENH Avoid potential data disclosure #233
Conversation
| const type = types.hasOwnProperty(data[linkID]?.typeKey) ? types[data[linkID]?.typeKey] : {}; | ||
| const type = types.hasOwnProperty(linkData.typeKey) ? types[linkData.typeKey] : {}; | ||
| links.push(<LinkPickerTitle | ||
| key={linkID} | ||
| id={linkID} | ||
| title={data[linkID]?.Title} | ||
| description={data[linkID]?.description} | ||
| versionState={data[linkID]?.versionState} | ||
| title={linkData.title} | ||
| description={linkData.description} | ||
| versionState={linkData.versionState} | ||
| typeTitle={type.title || ''} | ||
| typeIcon={type.icon} | ||
| onDelete={handleDelete} | ||
| onClick={() => { setEditingID(linkID); }} | ||
| onButtonKeyDownEdit={() => setIsKeyboardEditing(true)} | ||
| onUnpublishedVersionedState={handleUnpublishedVersionedState} | ||
| canDelete={data[linkID]?.canDelete ? true : false} | ||
| canDelete={linkData.canDelete ? true : false} |
There was a problem hiding this comment.
Unrelated refactoring - this variable is explicitly created above so a) let's use it and b) no need for null safety chaining operator.
| $data = $link->jsonSerialize(); | ||
| $data['canDelete'] = $link->canDelete(); | ||
| $data['description'] = $link->getDescription(); | ||
| $data['versionState'] = $link->getVersionedState(); | ||
| return $data; | ||
| return $link->getData(); |
There was a problem hiding this comment.
Moved these into the method on Link since the only time the method is called is here. Makes it easier to maintain and see the full list of what's passed through to the react component from PHP in one place.
| 'title' => $this->getTitle(), | ||
| 'description' => $this->getDescription(), | ||
| 'canDelete' => $this->canDelete(), | ||
| 'versionState' => $this->getVersionedState(), | ||
| 'typeKey' => $typeKey, |
There was a problem hiding this comment.
Explicitly only these are used in the component.
Even creating super-duper unique link subclasses won't require adding more data here - the modal is handled separately, this is just for display before opening the modal.
There's no way to easily customise the react component, so adding an extension hook to update this data is overkill.
| * Return a rendered version of this form. | ||
| * Return a rendered version of this link. | ||
| * | ||
| * This is returned when you access a form as $FormObject rather | ||
| * than <% with FormObject %> | ||
| * This is returned when you access a link as $LinkRelation or $Me rather | ||
| * than <% with LinkRelation %> |
There was a problem hiding this comment.
Unrelated refactoring - a link is not a form, this was probably just copy pasted from elsewhere originally.
GuySartorelli
force-pushed
the
pulls/4/refactor-jsonSerialize
branch
from
0da212f
to
b37cd55
Compare
| title={data[linkID]?.Title} | ||
| description={data[linkID]?.description} | ||
| versionState={data[linkID]?.versionState} | ||
| title={linkData.title} |
There was a problem hiding this comment.
Note upper case Title is now lower case title - all of the data is now using lowerCamelCase key names, as per javascript tradition. It was a mix before.
GuySartorelli
force-pushed
the
pulls/4/refactor-jsonSerialize
branch
from
b37cd55
to
2342c8d
Compare
|
Tested locally, works good |
Issue