Skip to content

Sas time encoding roundtrip#25524

Merged
rickle-msft merged 7 commits intoAzure:mainfrom
rickle-msft:sasTimeEncodingRoundtrip
Nov 30, 2021
Merged

Sas time encoding roundtrip#25524
rickle-msft merged 7 commits intoAzure:mainfrom
rickle-msft:sasTimeEncodingRoundtrip

Conversation

@rickle-msft
Copy link
Copy Markdown
Contributor

Resolves #22042

@ghost ghost added the Storage Storage Service (Queues, Blobs, Files) label Nov 19, 2021
@azure-sdk
Copy link
Copy Markdown
Collaborator

API changes have been detected in com.azure:azure-storage-common. You can review API changes here

API changes

-         public static OffsetDateTime parseDate(String dateString) 
+         public static TimeAndFormat parseDate(String dateString) 

@azure-sdk
Copy link
Copy Markdown
Collaborator

API changes have been detected in com.azure:azure-storage-common. You can review API changes here

API changes

+     public class TimeAndFormat {
+         public TimeAndFormat(OffsetDateTime dateTime, DateTimeFormatter formatter) 
+         public OffsetDateTime getDateTime() 
+         public DateTimeFormatter getFormatter() 
+     }
-         public static OffsetDateTime parseDate(String dateString) 
+         public static TimeAndFormat parseDate(String dateString) 
+         public static TimeAndFormat parseDateAndFormat(String dateString) 

* @throws IllegalArgumentException If {@code dateString} doesn't match an ISO8601 pattern
*/
public static OffsetDateTime parseDate(String dateString) {
public static TimeAndFormat parseDate(String dateString) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: mark parseDate as deprecated instead of changing the API

* @return the corresponding <code>Date</code> object
* @throws IllegalArgumentException If {@code dateString} doesn't match an ISO8601 pattern
*/
public static TimeAndFormat parseDateAndFormat(String dateString) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that azure-storage-common exposes it's implementation packages to the other azure-storage libraries we should think about moving this into implementation in case we need to do API changes to it in the future.

* or in a connection string, we have to preserve the formatting, especially the precision, so the signature still
* matches.
*/
public class TimeAndFormat {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about the new utility method, unless we're expecting to return this in public APIs somewhere. Also, let's make it final

@azure-sdk
Copy link
Copy Markdown
Collaborator

API changes have been detected in com.azure:azure-storage-common. You can review API changes here

API changes

+     public class TimeAndFormat {
+         public TimeAndFormat(OffsetDateTime dateTime, DateTimeFormatter formatter) 
+         public OffsetDateTime getDateTime() 
+         public DateTimeFormatter getFormatter() 
+     }
+         public static TimeAndFormat parseDateAndFormat(String dateString) 

@azure-sdk
Copy link
Copy Markdown
Collaborator

API changes have been detected in com.azure:azure-storage-common. You can review API changes here

API changes

+     public final class TimeAndFormat {
+         public TimeAndFormat(OffsetDateTime dateTime, DateTimeFormatter formatter) 
+         public OffsetDateTime getDateTime() 
+         public DateTimeFormatter getFormatter() 
+     }
-         public static OffsetDateTime parseDate(String dateString) 
+         @Deprecated public static OffsetDateTime parseDate(String dateString) 

* or in a connection string, we have to preserve the formatting, especially the precision, so the signature still
* matches.
*/
public final class TimeAndFormat {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this class isn't exposed in any public APIs does it itself need to be exposed as a public API?

@azure-sdk
Copy link
Copy Markdown
Collaborator

API changes have been detected in com.azure:azure-storage-common. You can review API changes here

API changes

-         public static OffsetDateTime parseDate(String dateString) 
+         @Deprecated public static OffsetDateTime parseDate(String dateString) 

@rickle-msft rickle-msft merged commit f254a1b into Azure:main Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] SAS token SAS_EXPIRY_TIME encoding results in an invalid signature

3 participants