Edge case where GEO is defined but not added due to LOCATION title not present

[titanism]

There is currently a bug here

ical-generator/src/event.ts

Lines 1753 to 1774 in 9190c84

	// LOCATION
	if (this.data.location?.title) {
	g += 'LOCATION:' + escape(
	this.data.location.title +
	(this.data.location.address ? '\n' + this.data.location.address : ''),
	false
	) + '\r\n';
	if (this.data.location.radius && this.data.location.geo) {
	g += 'X-APPLE-STRUCTURED-LOCATION;VALUE=URI;' +
	(this.data.location.address ? 'X-ADDRESS=' + escape(this.data.location.address, false) + ';' : '') +
	'X-APPLE-RADIUS=' + escape(this.data.location.radius, false) + ';' +
	'X-TITLE=' + escape(this.data.location.title, false) +
	':geo:' + escape(this.data.location.geo?.lat, false) + ',' +
	escape(this.data.location.geo?.lon, false) + '\r\n';
	}
	if (this.data.location.geo) {
	g += 'GEO:' + escape(this.data.location.geo?.lat, false) + ';' +
	escape(this.data.location.geo?.lon, false) + '\r\n';
	}
	}

.

As you can see, even if you provide a geo, because the title is not present, it won't get added to the output, and thus GEO won't be present due to missing LOCATION.

[sebbo2002]

I don't think I quite understand what you mean. Can you please post a concrete code example that does not work as desired?

[titanism]

Hi there 👋 Yes, to bring more clarity to this – you can have location = { geo: { lat: 20, lon: 20 } } without a title and the GEO property won't be added because the only time it would get added is if title was specified. The conditional on line 1754 is the culprit. Lines 1770 to 1773 should be moved outside of this conditional statement.

[titanism]

Here's a diff for you, if you want us to submit a PR with this let us know otherwise it's easy to implement:

        // LOCATION
        if (this.data.location?.title) {
            g += 'LOCATION:' + escape(
                this.data.location.title +
                (this.data.location.address ? '\n' + this.data.location.address : ''),
                false
            ) + '\r\n';

            if (this.data.location.radius && this.data.location.geo) {
                g += 'X-APPLE-STRUCTURED-LOCATION;VALUE=URI;' +
                    (this.data.location.address ? 'X-ADDRESS=' + escape(this.data.location.address, false) + ';' : '') +
                    'X-APPLE-RADIUS=' + escape(this.data.location.radius, false) + ';' +
                    'X-TITLE=' + escape(this.data.location.title, false) +
                    ':geo:' + escape(this.data.location.geo?.lat, false) + ',' +
                    escape(this.data.location.geo?.lon, false) + '\r\n';
            }

-            if (this.data.location.geo) {
-                g += 'GEO:' + escape(this.data.location.geo?.lat, false) + ';' +
-                    escape(this.data.location.geo?.lon, false) + '\r\n';
-            }
        }

+        if (this.data.location.geo) {
+            g += 'GEO:' + escape(this.data.location.geo?.lat, false) + ';' +
+                escape(this.data.location.geo?.lon, false) + '\r\n';
+        }

[sebbo2002]

Can you please go into more detail about how you set a location without a title? This should not actually be possible. As you can see here, the title is a mandatory attribute and not optional. In my experiment, a correct error message is also displayed when I try something else:

Example Code:

import ical from 'ical-generator';

const cal = ical();
const event = cal.createEvent({
  start: new Date()
});

event.location({
  geo: {
    lat: 52.51147570081018,
    lon: 13.342200696373846
  }
});

cal.toString();

Output:

Error: `location` isn't formatted correctly. See https://sebbo2002.github.io/ical-generator/develop/reference/classes/ICalEvent.html#location

[titanism]

This is strictly for the example of generating a ICS file from an object (e.g. one stored in a database).

If there is no LOCATION property, but a GEO is there, then the object in the database would have geo but not such values for the title/location/address/etc for the LOCATION.

As far as I know, the RFC spec doesn't have the GEO being dependent upon a LOCATION.

[titanism]

Another example... in case it's not obvious:

const databaseObject = {
  // ...
  location: {
    geo: {
      lat: 20,
      lon: 20
    }
  }
};

const cal = ical(databaseObject);
console.log(cal.toString());

The output won't have GEO, despite GEO existing in the database having been previously parsed (e.g. using a library such as node-ical).

[titanism]

Another example in case it's not obvious:

Input:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//sebbo.net//ical-generator.tests//EN
METHOD:ADD
X-BAZ:Boop
BEGIN:VEVENT
UID:123
SEQUENCE:0
DTSTAMP:20131004T233453Z
DTSTART;VALUE=DATE:20131004
DTEND;VALUE=DATE:20131006
X-MICROSOFT-CDO-ALLDAYEVENT:TRUE
X-MICROSOFT-MSNCALENDAR-ALLDAYEVENT:TRUE
SUMMARY:Sample Event
GEO:52.50363;13.32865
ORGANIZER;CN="Sebastian Pekarek";SENT-BY="foo@gmail.com":mailto:mail@sebbo.net
CATEGORIES:WORK
URL;VALUE=URI:http://sebbo.net/
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;DELEGATED-FROM="matt@examp
 le.com";CN="John":MAILTO:john@example.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=DELEGATED;DELEGATED-TO="john@exampl
 e.com";CN="Smith, Matt; (\"Sales\")":MAILTO:matt@example.com
ATTACH:https://files.sebbo.net/calendar/attachments/foo
STATUS:CONFIRMED
END:VEVENT
END:VCALENDAR

Script:

const cal = require('node-ical');

console.log(JSON.stringify(cal.sync.parseFile('test.ics'), null, 2));

Output:

{
  "123": {
    "type": "VEVENT",
    "params": [],
    "uid": "123",
    "sequence": "0",
    "dtstamp": "2013-10-04T23:34:53.000Z",
    "start": "2013-10-04T05:00:00.000Z",
    "datetype": "date",
    "end": "2013-10-06T05:00:00.000Z",
    "MICROSOFT-CDO-ALLDAYEVENT": "TRUE",
    "MICROSOFT-MSNCALENDAR-ALLDAYEVENT": "TRUE",
    "summary": "Sample Event",
    "geo": {
      "lat": 52.50363,
      "lon": 13.32865
    },
    "organizer": {
      "params": {
        "CN": "Sebastian Pekarek",
        "SENT-BY": "foo@gmail.com"
      },
      "val": "mailto:mail@sebbo.net"
    },
    "categories": [
      "WORK"
    ],
    "url": {
      "params": {
        "VALUE": "URI"
      },
      "val": "http://sebbo.net/"
    },
    "attendee": {
      "params": {
        "ROLE": "REQ-PARTICIPANT",
        "PARTSTAT": "ACCEPTED",
        "DELEGATED-FROM": "matt@example.com",
        "CN": "John"
      },
      "val": "MAILTO:john@example.com"
    },
    "attach": "https://files.sebbo.net/calendar/attachments/foo",
    "status": "CONFIRMED",
    "method": "ADD"
  },
  "vcalendar": {
    "type": "VCALENDAR",
    "version": "2.0",
    "prodid": "-//sebbo.net//ical-generator.tests//EN",
    "method": "ADD",
    "BAZ": "Boop"
  }
}

As you can see there's no LOCATION parsed, so there would be no title to give to this library, and thus GEO wouldn't get added even though it should (since GEO isn't dependent upon LOCATION).

Please fix this or we can submit a PR if you'd like.

Thank you for your time.

[sebbo2002]

This is strictly for the example of generating a ICS file from an object (e.g. one stored in a database).

Yes, but even then: For this library the title is a mandatory field and no output will be generated:

Example Code:

import ical from 'ical-generator';

const someDatabaseObject = {
  events: [
    {
      start: new Date(),
      location: {
        geo: {
          lat: 52.51147570081018,
          lon: 13.342200696373846
        }
      }
    }
  ]
};

const cal = ical(someDatabaseObject);
cal.toString();

Output:

Error: `location` isn't formatted correctly. See https://sebbo2002.github.io/ical-generator/develop/reference/classes/ICalEvent.html#location

Please run your example to confirm.

As far as I know, the RFC spec doesn't have the GEO being dependent upon a LOCATION.

This library produces RFC-compliant output, but as a developer I have a certain amount of leeway and don't have to allow every possibility in this API. On the contrary, I think a clear API helps many people to use this library more easily.

e.g. using a library such as node-ical

The object format that may be passed to generate a calendar is precisely documented and differs from the JSON that node-ical generates when parsing. Therefore, the output of node-ical cannot be transferred directly to ical-generator. I recommend not to store the ical in the database, but the corresponding JSON representation, which can be generated with toJSON():

Example Code:

import ical from 'ical-generator';

const calA = ical({
  events: [
    {
      start: new Date()
    }
  ]
});

const savedInDb = calA.toJSON();

const calB = ical(savedInDb);
calB.toString();

Output:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//sebbo.net//ical-generator//EN
BEGIN:VEVENT
UID:e6392bc0-45a6-4d5f-96b1-cb70044faeef
SEQUENCE:0
DTSTAMP:20240214T213111Z
DTSTART:20240214T213111Z
SUMMARY:
END:VEVENT
END:VCALENDAR

[titanism]

Not everyone is using this library by itself, often times folks use this to generate an ICS file from data obtained elsewhere.

For the purposes of generating an ICS file, which this library implies it offers, it shouldn't arbitrarily deviate from the specification and require that in order to have GEO in your output, you must supply a LOCATION.

Users like ourselves are now having to put in values like _ for the title as a placeholder (or) do hacky things like replace the LOCATION line that gets added after like LOCATION:_.

We need to use the toString() method in order to build ICS files because we are operating a CalDAV server.

[titanism]

@sebbo2002 Could you please merge #570 and release to npm? Thank you 🙏

[sebbo2002]

For the purposes of generating an ICS file, which this library implies it offers, it shouldn't arbitrarily deviate from the specification and require that in order to have GEO in your output, you must supply a LOCATION.

Please don't get me wrong, I think it's a good idea if we make the title optional in ical-generator. For me it was just important to understand if ical-generator currently pretends to support this somehow and only the generation of the ics is a problem - as it is implied in the PR description - or if it is currently not possible to specify a location without a title and therefore it is a technically new feature. The latter seems to be the case, that was my point. I apologize if there was any confusion.

If there are no objections, I would close this ticket now and clarify everything else in #570 so that there are no duplicate discussions on the same topic. Please let me know if there is still something open that is not part of #570.

[sebbo2002]

🎉 This issue has been resolved in version 7.0.0-develop.1 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release
• v7.0.0-develop.1

Your semantic-release bot 📦🚀

[sebbo2002]

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release
• v7.0.0

Your semantic-release bot 📦🚀