Skip to content

Commit

Permalink
fix possible key duplication in ReferencesHandler (yWorks#188)
Browse files Browse the repository at this point in the history
  • Loading branch information
HackbrettXXX authored Jul 20, 2021
1 parent 8eccf90 commit 5909601
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 6 deletions.
9 changes: 6 additions & 3 deletions src/context/referenceshandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ import { RGBColor } from '../utils/rgbcolor'
export class ReferencesHandler {
private readonly renderedElements: { [key: string]: SvgNode }
private readonly idMap: { [id: string]: SvgNode }
private readonly idPrefix: string
private static instanceCounter = 0

constructor(idMap: { [id: string]: SvgNode }) {
this.renderedElements = {}
this.idMap = idMap
this.idPrefix = String(ReferencesHandler.instanceCounter++)
}

public async getRendered(
id: string,
color: RGBColor | null,
renderCallback: (node: SvgNode) => Promise<void>
): Promise<SvgNode> {
const key = ReferencesHandler.generateKey(id, color)
const key = this.generateKey(id, color)
if (this.renderedElements.hasOwnProperty(key)) {
return this.renderedElements[id]
}
Expand All @@ -33,7 +36,7 @@ export class ReferencesHandler {
return this.idMap[cssEsc(id, { isIdentifier: true })]
}

public static generateKey(id: string, color: RGBColor | null): string {
return id + '|' + (color || new RGBColor('rgb(0,0,0)')).toRGBA()
public generateKey(id: string, color: RGBColor | null): string {
return this.idPrefix + '|' + id + '|' + (color || new RGBColor('rgb(0,0,0)')).toRGBA()
}
}
5 changes: 2 additions & 3 deletions src/nodes/use.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { SvgNode } from './svgnode'
import { Symbol } from './symbol'
import { Viewport } from '../context/viewport'
import { RGBColor } from '../utils/rgbcolor'
import { ReferencesHandler } from '../context/referenceshandler'

/**
* Draws the element referenced by a use node, makes use of pdf's XObjects/FormObjects so nodes are only written once
Expand Down Expand Up @@ -86,7 +85,7 @@ export class Use extends GraphicsNode {
context.pdf.clip().discardPath()
}

context.pdf.doFormObject(ReferencesHandler.generateKey(id, color), t)
context.pdf.doFormObject(context.refsHandler.generateKey(id, color), t)
context.pdf.restoreGraphicsState()
}

Expand All @@ -112,7 +111,7 @@ export class Use extends GraphicsNode {
} else {
await node.render(refContext)
}
refContext.pdf.endFormObject(ReferencesHandler.generateKey(id, color))
refContext.pdf.endFormObject(refContext.refsHandler.generateKey(id, color))
}

protected getBoundingBoxCore(context: Context): number[] {
Expand Down
1 change: 1 addition & 0 deletions test/common/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ window.tests = [
'complete-social-network',
'custom-fonts',
'display-none-and-visibility-inheritance',
'duplicate-ids',
'empty-elements',
'fill-and-stroke-inheritance',
'fill-and-stroke-opacity',
Expand Down
Binary file added test/specs/duplicate-ids/reference.pdf
Binary file not shown.
9 changes: 9 additions & 0 deletions test/specs/duplicate-ids/spec.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 5909601

Please sign in to comment.